Avoid zmalloc_size() in kvobjAllocSize() and approximate instead#14704
Avoid zmalloc_size() in kvobjAllocSize() and approximate instead#14704skaslev wants to merge 1 commit intoredis:unstablefrom
Conversation
|
Hi, I’m Jit, a friendly security platform designed to help developers build secure applications from day zero with an MVS (Minimal viable security) mindset. In case there are security findings, they will be communicated to you as a comment inside the PR. Hope you’ll enjoy using Jit. Questions? Comments? Want to learn more? Get in touch with us. |
❌ Security scan failedSecurity scan failed: Branch no-zmalloc-size does not exist in the remote repository 💡 Need to bypass this check? Comment |
🤖 Augment PR SummarySummary: Replaces 🤖 Was this summary useful? React with 👍 or 👎 |
| size_t kvobjAllocSize(kvobj *o) { | ||
| /* All kv-objects has at least kvobj header and embedded key */ | ||
| size_t asize = zmalloc_size(kvobjGetAllocPtr(o)); | ||
| debugServerAssert(o->iskvobj); |
There was a problem hiding this comment.
Since robj and kvobj are typedefs of the same underlying struct, kvobjAllocSize() can be called with iskvobj==0; with debugServerAssert compiled out, kvobjGetKey()/sdsAllocSize() will read past the allocation and can crash.
Consider an always-on guard (e.g., serverAssert) or an unlikely(!o->iskvobj) safe fallback for release builds.
🤖 Was this useful? React with 👍 or 👎
CE Performance Automation : step 1 of 2 (build) STARTING...This comment was automatically generated given a benchmark was triggered.
|
CE Performance Automation : step 2 of 2 (benchmark) RUNNING...This comment was automatically generated given a benchmark was triggered. Started benchmark suite at 2026-01-17 18:08:52.477198 and took 101.54245 seconds up until now. In total will run 273 benchmarks. |
| @@ -1236,8 +1236,16 @@ size_t kvobjComputeSize(robj *key, kvobj *o, size_t sample_size, int dbid) { | |||
| } | |||
|
|
|||
| size_t kvobjAllocSize(kvobj *o) { | |||
There was a problem hiding this comment.
Add a comment explaining that this returns the requested size, not the actual allocated size.
Also verify that using here requested instead of allocated doesn't create kind of drift such that at one place we rely on allocation size and on another place on requested size.
Avoid zmalloc_size() in kvobjAllocSize() and use approximation instead.
Since for ongoing key allocation histograms work (#14695) we need to call
kvobjAllocSize() more often on hot paths, using zmalloc_size() would cause
unnecessary performance overhead.
Note
Optimizes memory usage accounting for kv objects by avoiding allocator-sized queries on hot paths.
kvobjAllocSizeto approximate allocation size: sumssizeof(kvobj), metadata slots, embedded key header/SDS, and embedded string (forOBJ_ENCODING_EMBSTR), then adds per-type sizesdebugServerAssert(o->iskvobj)to ensure the function is used only with kv objectsWritten by Cursor Bugbot for commit 24f6116. This will update automatically on new commits. Configure here.