Fix UBSan error in stream trim when processing last entry#14669
Merged
sundb merged 1 commit intoredis:unstablefrom Jan 7, 2026
Merged
Fix UBSan error in stream trim when processing last entry#14669sundb merged 1 commit intoredis:unstablefrom
sundb merged 1 commit intoredis:unstablefrom
Conversation
skaslev
approved these changes
Jan 7, 2026
sggeorgiev
reviewed
Jan 7, 2026
| intptr_t delta = p - lp; | ||
| intptr_t delta = p ? (p - lp) : 0; /* p may be NULL if this was the last entry */ | ||
| flags |= STREAM_ITEM_FLAG_DELETED; | ||
| lp = lpReplaceInteger(lp, &pcopy, flags); |
Collaborator
There was a problem hiding this comment.
It looks like lpReplaceInteger doesn't modify the lp because the size of flags is same. In this case you can remove delta and updating of p.
Collaborator
Author
There was a problem hiding this comment.
lpReplaceInteger will still change its size because the encoding is determined by the size of lval, not the type of lval. However, in theory, the length will not change afte | STREAM_ITEM_FLAG_DELETED(1), but i still prefer to keep it.
sggeorgiev
approved these changes
Jan 7, 2026
Collaborator
Author
|
Fully CI: https://github.com/sundb/redis/actions/runs/20770473823 |
sundb
added a commit
to sundb/redis
that referenced
this pull request
Jan 27, 2026
## Summary This bus was introduced by redis#14623 Before PR redis#14623, when a stream node was going to be fully removed, we would just delete the whole node directly instead of iterating through and deleting each entry. Now, with the XTRIM/XADD flags, we have to iterate and delete entries one by one. However, the implementation in issue redis#8169 didn’t consider the case where all entries are removed, so `p` can end up being NULL. Fixes an UndefinedBehaviorSanitizer error in `streamTrim()` when marking the last entry in a listpack as deleted. The issue occurs when performing pointer arithmetic on a NULL pointer after `lpNext()` reaches the end of the listpack. ## Solution If p is NULL, we skip the delta calculation and the calculation of new `p`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This bus was introduced by #14623
Before PR #14623, when a stream node was going to be fully removed, we would just delete the whole node directly instead of iterating through and deleting each entry.
Now, with the XTRIM / XADD flags, we have to iterate and delete entries one by one. However, the implementation in issue #8169 didn’t consider the case where all entries are removed, so p can end up being NULL.
Fixes an UndefinedBehaviorSanitizer error in
streamTrim()when marking the last entry in a listpack as deleted. The issue occurs when performing pointer arithmetic on a NULL pointer afterlpNext()reaches the end of the listpack.Solution
If p is NULL, we skip the delta calculation and skip the calculation of new
p.Failed CI: https://github.com/redis/redis/actions/runs/20766104241/job/59632440335