Improve refcount check in 'decrRefCount' (#13888)
CI / test-ubuntu-latest (push) Has been cancelled Details
CI / test-sanitizer-address (push) Has been cancelled Details
CI / build-debian-old (push) Has been cancelled Details
CI / build-macos-latest (push) Has been cancelled Details
CI / build-32bit (push) Has been cancelled Details
CI / build-libc-malloc (push) Has been cancelled Details
CI / build-centos-jemalloc (push) Has been cancelled Details
CI / build-old-chain-jemalloc (push) Has been cancelled Details
Codecov / code-coverage (push) Has been cancelled Details
External Server Tests / test-external-standalone (push) Has been cancelled Details
External Server Tests / test-external-cluster (push) Has been cancelled Details
External Server Tests / test-external-nodebug (push) Has been cancelled Details
Spellcheck / Spellcheck (push) Has been cancelled Details

The code of 'decrRefCount' included a validity check that would panic
the server if the refcount ever became invalid. However, due to the way
it was written, this could only happen if a corrupted value was written
to the field, or we attempted to decrement a newly-allocated and
never-incremented object. Incorrectly-tracked refcounts would not be
caught, as the code would never actually reduce the refcount from 1 to
0. This left potential use-after-free errors unhandled.

Improved the code so that incorrect tracking of refcounts causes a
panic, even if the freed memory happens to still be owned by the
application and not re-allocated.
This commit is contained in:
Slava Koyfman 2025-04-03 16:29:06 +03:00 committed by GitHub
parent 3cdb8c6046
commit fd4b5cb3fa
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 9 additions and 4 deletions

View File

@ -359,7 +359,15 @@ void incrRefCount(robj *o) {
}
void decrRefCount(robj *o) {
if (o->refcount == 1) {
if (o->refcount == OBJ_SHARED_REFCOUNT)
return; /* Nothing to do: this refcount is immutable. */
if (unlikely(o->refcount <= 0)) {
serverPanic("illegal decrRefCount for object with: type %u, encoding %u, refcount %d",
o->type, o->encoding, o->refcount);
}
if (--(o->refcount) == 0) {
switch(o->type) {
case OBJ_STRING: freeStringObject(o); break;
case OBJ_LIST: freeListObject(o); break;
@ -371,9 +379,6 @@ void decrRefCount(robj *o) {
default: serverPanic("Unknown object type"); break;
}
zfree(o);
} else {
if (o->refcount <= 0) serverPanic("decrRefCount against refcount <= 0");
if (o->refcount != OBJ_SHARED_REFCOUNT) o->refcount--;
}
}