Skip to content

avoid possible use-after-free with module KSN changes #13875

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
oranagra merged 1 commit into from
Mar 24, 2025
Merged

avoid possible use-after-free with module KSN changes #13875

oranagra merged 1 commit into from
Mar 24, 2025

Conversation

@oranagra
Copy link
Member

@oranagra oranagra commented Mar 24, 2025

in #13505, we changed the code to use the string value of the key rather than the integer value on the stack, but we have a test in unit/moduleapi/keyspace_events that uses keyspace notification hook to modify the value with RM_StringDMA, which can cause this value to be released before used. the reason it didn't happen so far is because we were using shared integers, so releasing the object doesn't free it.

@oranagra
avoid possible use-after-free with module KSN changes
fe4b243 
in redis#13505, we changed the code to use the string value of the key rather
than the integer value on the stack, but we have a test in
unit/moduleapi/keyspace_events that uses keyspace notification hook to
modify the value with RM_StringDMA, which can cause this value to be
released before used. the reason it didn't happen so far is because we
were using shared integers, so releasing the object doesn't free it.
@oranagra oranagra requested a review from sundb March 24, 2025 07:43
@oranagra
Copy link
Member Author

oranagra commented Mar 24, 2025 

==2256913==ERROR: AddressSanitizer: heap-use-after-free on address 0x60400016e710 at pc 0x562bbd9eaba8 bp 0x7ffd43a335b0 sp 0x7ffd43a335a0
READ of size 1 at 0x60400016e710 thread T0
    #0 0x562bbd9eaba7 in addReply /home/oran/work/RedisLabs/garantia.redis/src/networking.c:480
    #1 0x562bbdac79fe in addReplyLongLongFromStr /home/oran/work/RedisLabs/garantia.redis/src/networking.c:1085
    #2 0x562bbdac79fe in incrDecrCommand /home/oran/work/RedisLabs/garantia.redis/src/t_string.c:609
    #3 0x562bbd977a52 in call /home/oran/work/RedisLabs/garantia.redis/src/server.c:5440
    #4 0x562bbdcc4086 in moduleCallArgvGeneric /home/oran/work/RedisLabs/garantia.redis/src/module.c:6749
    #5 0x562bbdcc62ee in moduleCallFormat /home/oran/work/RedisLabs/garantia.redis/src/module.c:6794
    #6 0x562bbdcc62ee in RM_Call /home/oran/work/RedisLabs/garantia.redis/src/module.c:6915
    #7 0x7f0953be3244 in KeySpace_PostNotificationString /home/oran/work/RedisLabs/garantia.redis/tests/modules/postnotifications.c:47
    #8 0x562bbdce1dce in firePostExecutionUnitJobs /home/oran/work/RedisLabs/garantia.redis/src/module.c:9370
    #9 0x562bbd976ce4 in postExecutionUnitOperationsEx /home/oran/work/RedisLabs/garantia.redis/src/server.c:5310
    #10 0x562bbd976ce4 in postExecutionUnitOperationsEx /home/oran/work/RedisLabs/garantia.redis/src/server.c:5306
    #11 0x562bbd976ce4 in afterCommandEx /home/oran/work/RedisLabs/garantia.redis/src/server.c:5715
    #12 0x562bbd976ce4 in call /home/oran/work/RedisLabs/garantia.redis/src/server.c:5631
    #13 0x562bbd9783f2 in executeCommand /home/oran/work/RedisLabs/garantia.redis/src/server.c:6243
    #14 0x562bbd97c048 in processCommand /home/oran/work/RedisLabs/garantia.redis/src/server.c:5931
    #15 0x562bbd9e6926 in processCommandAndResetClient /home/oran/work/RedisLabs/garantia.redis/src/networking.c:2897
    #16 0x562bbd9e6926 in processInputBuffer /home/oran/work/RedisLabs/garantia.redis/src/networking.c:3085
    #17 0x562bbd9f6772 in readQueryFromClient /home/oran/work/RedisLabs/garantia.redis/src/networking.c:3294

0x60400016e710 is located 0 bytes inside of 40-byte region [0x60400016e710,0x60400016e738)
freed by thread T0 here:
    #0 0x7f095c2fa537 in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:127
    #1 0x562bbda216af in dbSetValue /home/oran/work/RedisLabs/garantia.redis/src/db.c:645
    #2 0x562bbda25233 in dbReplaceValueWithDictEntry /home/oran/work/RedisLabs/garantia.redis/src/db.c:708
    #3 0x562bbda25233 in dbUnshareStringValueWithDictEntry /home/oran/work/RedisLabs/garantia.redis/src/db.c:1199
    #4 0x562bbda25233 in dbUnshareStringValueWithDictEntry /home/oran/work/RedisLabs/garantia.redis/src/db.c:1187
    #5 0x562bbdcb351c in dbUnshareStringValue /home/oran/work/RedisLabs/garantia.redis/src/db.c:1182
    #6 0x562bbdcb351c in RM_StringDMA /home/oran/work/RedisLabs/garantia.redis/src/module.c:4580
    #7 0x7f0953bd0268 in KeySpace_NotificationModuleString /home/oran/work/RedisLabs/garantia.redis/tests/modules/keyspace_events.c:122

@oranagra oranagra merged commit 2a18970 into redis:unstable Mar 24, 2025
18 checks passed
@oranagra oranagra deleted the fix_use_after_free_in_module_test branch March 24, 2025 10:24
funny-dog pushed a commit to funny-dog/redis that referenced this pull request Sep 17, 2025
@oranagra
avoid possible use-after-free with module KSN changes (redis#13875)
0fd6814 
in redis#13505, we changed the code to use the string value of the key rather
than the integer value on the stack, but we have a test in
unit/moduleapi/keyspace_events that uses keyspace notification hook to
modify the value with RM_StringDMA, which can cause this value to be
released before used. the reason it didn't happen so far is because we
were using shared integers, so releasing the object doesn't free it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sundb sundb sundb approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@oranagra @sundb