Skip to content

Handle NULL pointer in streamTrim listpack delta calculation#3591

Merged
enjoy-binbin merged 1 commit into
valkey-io:unstablefrom
smkher:fix/stream-trim-listpack-corruption
May 1, 2026
Merged

Handle NULL pointer in streamTrim listpack delta calculation#3591
enjoy-binbin merged 1 commit into
valkey-io:unstablefrom
smkher:fix/stream-trim-listpack-corruption

Conversation

@smkher

@smkher smkher commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

When XTRIM marks the last entry in a listpack node as deleted, lpNext() returns NULL after the lp-count field (EOF). The delta calculation (p - lp) on a NULL pointer is undefined behavior and produces a garbage pointer, corrupting the listpack. A subsequent XREAD hitting the corrupted node triggers the lpValidateNext assertion failure and crashes the server.

Guard the delta calculation with a NULL check so the while(p) loop terminates naturally when the last entry is reached.

Fixes #3569

@smkher smkher force-pushed the fix/stream-trim-listpack-corruption branch from fd8945a to 322cca4 Compare April 29, 2026 22:43
@codecov

codecov Bot commented Apr 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.62%. Comparing base (678a06d) to head (0ab9055).
⚠️ Report is 7 commits behind head on unstable.

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #3591      +/-   ##
============================================
- Coverage     76.63%   76.62%   -0.01%     
============================================
  Files           160      160              
  Lines         80472    80472              
============================================
- Hits          61668    61661       -7     
- Misses        18804    18811       +7     
Files with missing lines Coverage Δ
src/t_stream.c 94.45% <100.00%> (+0.20%) ⬆️

... and 15 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@rainsupreme rainsupreme left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks clean, and the test case is straightforward too! I only came up with a couple nits - this looks good to me 🙂

Comment thread src/t_stream.c Outdated
Comment thread src/t_stream.c Outdated

@sarthakaggarwal97 sarthakaggarwal97 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me! Nice fix.

@sarthakaggarwal97 sarthakaggarwal97 added the run-extra-tests Run extra tests on this PR (Runs all tests from daily except valgrind and RESP) label Apr 29, 2026
Comment thread src/t_stream.c Outdated
@smkher smkher force-pushed the fix/stream-trim-listpack-corruption branch from 322cca4 to 909956b Compare April 30, 2026 03:17
When XTRIM marks the last entry in a listpack node as deleted,
lpNext() returns NULL after the lp-count field (EOF). The delta
calculation (p - lp) on a NULL pointer is undefined behavior and
produces a garbage pointer, corrupting the listpack. A subsequent
XREAD hitting the corrupted node triggers the lpValidateNext
assertion failure and crashes the server.

Guard the delta calculation with a NULL check so the while(p) loop
terminates naturally when the last entry is reached.

Signed-off-by: Saurabh Kher <saurabh@amazon.com>
@smkher smkher force-pushed the fix/stream-trim-listpack-corruption branch from 909956b to 0ab9055 Compare April 30, 2026 03:23

@roshkhatri roshkhatri left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good, Thank you @smkher

@enjoy-binbin enjoy-binbin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks.

@enjoy-binbin enjoy-binbin changed the title fix: Handle NULL pointer in streamTrim listpack delta calculation Handle NULL pointer in streamTrim listpack delta calculation Apr 30, 2026
@enjoy-binbin

Copy link
Copy Markdown
Member

So i suppose it affect all the versions, like 7.2/8.0/8.1/... ?

@enjoy-binbin enjoy-binbin added the release-notes This issue should get a line item in the release notes label Apr 30, 2026
@sarthakaggarwal97

Copy link
Copy Markdown
Contributor

Yes we should backport this change to those branches.

@sarthakaggarwal97

Copy link
Copy Markdown
Contributor

Provenance Guard is failing due to a wrong commit. It was fixed by Ping in #3594

@smkher

smkher commented Apr 30, 2026

Copy link
Copy Markdown
Contributor Author

@enjoy-binbin @sarthakaggarwal97 The fix applies to all of 7.2, 8.0, 8.1, 9.0, and 9.1. Adding this issue to the respective folders for backport

@roshkhatri roshkhatri moved this to To be backported in Valkey 7.2 Apr 30, 2026
@roshkhatri roshkhatri moved this to To be backported in Valkey 8.0 Apr 30, 2026
@roshkhatri roshkhatri moved this to To be backported in Valkey 8.1 Apr 30, 2026
@roshkhatri roshkhatri moved this to To be backported in Valkey 9.0 Apr 30, 2026
@roshkhatri roshkhatri moved this to To be backported in Valkey 9.1 Apr 30, 2026
sarthakaggarwal97 pushed a commit to sarthakaggarwal97/valkey that referenced this pull request May 7, 2026
…io#3591)

When XTRIM marks the last entry in a listpack node as deleted, lpNext()
returns NULL after the lp-count field (EOF). The delta calculation (p -
lp) on a NULL pointer is undefined behavior and produces a garbage
pointer, corrupting the listpack. A subsequent XREAD hitting the
corrupted node triggers the lpValidateNext assertion failure and crashes
the server.

Guard the delta calculation with a NULL check so the while(p) loop
terminates naturally when the last entry is reached.

Fixes valkey-io#3569

Signed-off-by: Saurabh Kher <saurabh@amazon.com>
Co-authored-by: Saurabh Kher <saurabh@amazon.com>
sarthakaggarwal97 pushed a commit to sarthakaggarwal97/valkey that referenced this pull request May 7, 2026
…io#3591)

When XTRIM marks the last entry in a listpack node as deleted, lpNext()
returns NULL after the lp-count field (EOF). The delta calculation (p -
lp) on a NULL pointer is undefined behavior and produces a garbage
pointer, corrupting the listpack. A subsequent XREAD hitting the
corrupted node triggers the lpValidateNext assertion failure and crashes
the server.

Guard the delta calculation with a NULL check so the while(p) loop
terminates naturally when the last entry is reached.

Fixes valkey-io#3569

Signed-off-by: Saurabh Kher <saurabh@amazon.com>
Co-authored-by: Saurabh Kher <saurabh@amazon.com>
sarthakaggarwal97 pushed a commit to sarthakaggarwal97/valkey that referenced this pull request May 7, 2026
…io#3591)

When XTRIM marks the last entry in a listpack node as deleted, lpNext()
returns NULL after the lp-count field (EOF). The delta calculation (p -
lp) on a NULL pointer is undefined behavior and produces a garbage
pointer, corrupting the listpack. A subsequent XREAD hitting the
corrupted node triggers the lpValidateNext assertion failure and crashes
the server.

Guard the delta calculation with a NULL check so the while(p) loop
terminates naturally when the last entry is reached.

Fixes valkey-io#3569

Signed-off-by: Saurabh Kher <saurabh@amazon.com>
Co-authored-by: Saurabh Kher <saurabh@amazon.com>
sarthakaggarwal97 pushed a commit to sarthakaggarwal97/valkey that referenced this pull request May 7, 2026
…io#3591)

When XTRIM marks the last entry in a listpack node as deleted, lpNext()
returns NULL after the lp-count field (EOF). The delta calculation (p -
lp) on a NULL pointer is undefined behavior and produces a garbage
pointer, corrupting the listpack. A subsequent XREAD hitting the
corrupted node triggers the lpValidateNext assertion failure and crashes
the server.

Guard the delta calculation with a NULL check so the while(p) loop
terminates naturally when the last entry is reached.

Fixes valkey-io#3569

Signed-off-by: Saurabh Kher <saurabh@amazon.com>
Co-authored-by: Saurabh Kher <saurabh@amazon.com>
lucasyonge pushed a commit that referenced this pull request May 11, 2026
When XTRIM marks the last entry in a listpack node as deleted, lpNext()
returns NULL after the lp-count field (EOF). The delta calculation (p -
lp) on a NULL pointer is undefined behavior and produces a garbage
pointer, corrupting the listpack. A subsequent XREAD hitting the
corrupted node triggers the lpValidateNext assertion failure and crashes
the server.

Guard the delta calculation with a NULL check so the while(p) loop
terminates naturally when the last entry is reached.

Fixes #3569

Signed-off-by: Saurabh Kher <saurabh@amazon.com>
Co-authored-by: Saurabh Kher <saurabh@amazon.com>
lucasyonge pushed a commit that referenced this pull request May 12, 2026
When XTRIM marks the last entry in a listpack node as deleted, lpNext()
returns NULL after the lp-count field (EOF). The delta calculation (p -
lp) on a NULL pointer is undefined behavior and produces a garbage
pointer, corrupting the listpack. A subsequent XREAD hitting the
corrupted node triggers the lpValidateNext assertion failure and crashes
the server.

Guard the delta calculation with a NULL check so the while(p) loop
terminates naturally when the last entry is reached.

Fixes #3569

Signed-off-by: Saurabh Kher <saurabh@amazon.com>
Co-authored-by: Saurabh Kher <saurabh@amazon.com>
@sarthakaggarwal97 sarthakaggarwal97 moved this from To be backported to Done in Valkey 9.1 May 16, 2026
valkeyrie-ops Bot pushed a commit that referenced this pull request May 18, 2026
When XTRIM marks the last entry in a listpack node as deleted, lpNext()
returns NULL after the lp-count field (EOF). The delta calculation (p -
lp) on a NULL pointer is undefined behavior and produces a garbage
pointer, corrupting the listpack. A subsequent XREAD hitting the
corrupted node triggers the lpValidateNext assertion failure and crashes
the server.

Guard the delta calculation with a NULL check so the while(p) loop
terminates naturally when the last entry is reached.

Fixes #3569

Signed-off-by: Saurabh Kher <saurabh@amazon.com>
Co-authored-by: Saurabh Kher <saurabh@amazon.com>
valkeyrie-ops Bot pushed a commit that referenced this pull request May 18, 2026
When XTRIM marks the last entry in a listpack node as deleted, lpNext()
returns NULL after the lp-count field (EOF). The delta calculation (p -
lp) on a NULL pointer is undefined behavior and produces a garbage
pointer, corrupting the listpack. A subsequent XREAD hitting the
corrupted node triggers the lpValidateNext assertion failure and crashes
the server.

Guard the delta calculation with a NULL check so the while(p) loop
terminates naturally when the last entry is reached.

Fixes #3569

Signed-off-by: Saurabh Kher <saurabh@amazon.com>
Co-authored-by: Saurabh Kher <saurabh@amazon.com>
valkeyrie-ops Bot pushed a commit that referenced this pull request May 18, 2026
When XTRIM marks the last entry in a listpack node as deleted, lpNext()
returns NULL after the lp-count field (EOF). The delta calculation (p -
lp) on a NULL pointer is undefined behavior and produces a garbage
pointer, corrupting the listpack. A subsequent XREAD hitting the
corrupted node triggers the lpValidateNext assertion failure and crashes
the server.

Guard the delta calculation with a NULL check so the while(p) loop
terminates naturally when the last entry is reached.

Fixes #3569

Signed-off-by: Saurabh Kher <saurabh@amazon.com>
Co-authored-by: Saurabh Kher <saurabh@amazon.com>
valkeyrie-ops Bot pushed a commit that referenced this pull request May 20, 2026
When XTRIM marks the last entry in a listpack node as deleted, lpNext()
returns NULL after the lp-count field (EOF). The delta calculation (p -
lp) on a NULL pointer is undefined behavior and produces a garbage
pointer, corrupting the listpack. A subsequent XREAD hitting the
corrupted node triggers the lpValidateNext assertion failure and crashes
the server.

Guard the delta calculation with a NULL check so the while(p) loop
terminates naturally when the last entry is reached.

Fixes #3569

Signed-off-by: Saurabh Kher <saurabh@amazon.com>
Co-authored-by: Saurabh Kher <saurabh@amazon.com>
valkeyrie-ops Bot pushed a commit that referenced this pull request May 29, 2026
When XTRIM marks the last entry in a listpack node as deleted, lpNext()
returns NULL after the lp-count field (EOF). The delta calculation (p -
lp) on a NULL pointer is undefined behavior and produces a garbage
pointer, corrupting the listpack. A subsequent XREAD hitting the
corrupted node triggers the lpValidateNext assertion failure and crashes
the server.

Guard the delta calculation with a NULL check so the while(p) loop
terminates naturally when the last entry is reached.

Fixes #3569

Signed-off-by: Saurabh Kher <saurabh@amazon.com>
Co-authored-by: Saurabh Kher <saurabh@amazon.com>
zuiderkwast pushed a commit that referenced this pull request Jun 2, 2026
When XTRIM marks the last entry in a listpack node as deleted, lpNext()
returns NULL after the lp-count field (EOF). The delta calculation (p -
lp) on a NULL pointer is undefined behavior and produces a garbage
pointer, corrupting the listpack. A subsequent XREAD hitting the
corrupted node triggers the lpValidateNext assertion failure and crashes
the server.

Guard the delta calculation with a NULL check so the while(p) loop
terminates naturally when the last entry is reached.

Fixes #3569

Signed-off-by: Saurabh Kher <saurabh@amazon.com>
Co-authored-by: Saurabh Kher <saurabh@amazon.com>
@zuiderkwast zuiderkwast moved this from To be backported to 8.1.8 (WIP) in Valkey 8.1 Jun 2, 2026
zuiderkwast pushed a commit that referenced this pull request Jun 2, 2026
When XTRIM marks the last entry in a listpack node as deleted, lpNext()
returns NULL after the lp-count field (EOF). The delta calculation (p -
lp) on a NULL pointer is undefined behavior and produces a garbage
pointer, corrupting the listpack. A subsequent XREAD hitting the
corrupted node triggers the lpValidateNext assertion failure and crashes
the server.

Guard the delta calculation with a NULL check so the while(p) loop
terminates naturally when the last entry is reached.

Fixes #3569

Signed-off-by: Saurabh Kher <saurabh@amazon.com>
Co-authored-by: Saurabh Kher <saurabh@amazon.com>
valkeyrie-ops Bot pushed a commit that referenced this pull request Jun 3, 2026
When XTRIM marks the last entry in a listpack node as deleted, lpNext()
returns NULL after the lp-count field (EOF). The delta calculation (p -
lp) on a NULL pointer is undefined behavior and produces a garbage
pointer, corrupting the listpack. A subsequent XREAD hitting the
corrupted node triggers the lpValidateNext assertion failure and crashes
the server.

Guard the delta calculation with a NULL check so the while(p) loop
terminates naturally when the last entry is reached.

Fixes #3569

Signed-off-by: Saurabh Kher <saurabh@amazon.com>
Co-authored-by: Saurabh Kher <saurabh@amazon.com>
valkeyrie-ops Bot pushed a commit that referenced this pull request Jun 14, 2026
When XTRIM marks the last entry in a listpack node as deleted, lpNext()
returns NULL after the lp-count field (EOF). The delta calculation (p -
lp) on a NULL pointer is undefined behavior and produces a garbage
pointer, corrupting the listpack. A subsequent XREAD hitting the
corrupted node triggers the lpValidateNext assertion failure and crashes
the server.

Guard the delta calculation with a NULL check so the while(p) loop
terminates naturally when the last entry is reached.

Fixes #3569

Signed-off-by: Saurabh Kher <saurabh@amazon.com>
Co-authored-by: Saurabh Kher <saurabh@amazon.com>
sarthakaggarwal97 pushed a commit that referenced this pull request Jun 17, 2026
When XTRIM marks the last entry in a listpack node as deleted, lpNext()
returns NULL after the lp-count field (EOF). The delta calculation (p -
lp) on a NULL pointer is undefined behavior and produces a garbage
pointer, corrupting the listpack. A subsequent XREAD hitting the
corrupted node triggers the lpValidateNext assertion failure and crashes
the server.

Guard the delta calculation with a NULL check so the while(p) loop
terminates naturally when the last entry is reached.

Fixes #3569

Signed-off-by: Saurabh Kher <saurabh@amazon.com>
Co-authored-by: Saurabh Kher <saurabh@amazon.com>
@valkeyrie-ops valkeyrie-ops Bot moved this from To be backported to Done in Valkey 7.2 Jun 17, 2026
sarthakaggarwal97 pushed a commit that referenced this pull request Jun 18, 2026
When XTRIM marks the last entry in a listpack node as deleted, lpNext()
returns NULL after the lp-count field (EOF). The delta calculation (p -
lp) on a NULL pointer is undefined behavior and produces a garbage
pointer, corrupting the listpack. A subsequent XREAD hitting the
corrupted node triggers the lpValidateNext assertion failure and crashes
the server.

Guard the delta calculation with a NULL check so the while(p) loop
terminates naturally when the last entry is reached.

Fixes #3569

Signed-off-by: Saurabh Kher <saurabh@amazon.com>
Co-authored-by: Saurabh Kher <saurabh@amazon.com>
@valkeyrie-ops valkeyrie-ops Bot moved this from To be backported to Done in Valkey 8.0 Jun 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-notes This issue should get a line item in the release notes run-extra-tests Run extra tests on this PR (Runs all tests from daily except valgrind and RESP)

Projects

Status: Done
Status: Done
Status: 8.1.8
Status: To be backported
Status: Done

Development

Successfully merging this pull request may close these issues.

[CRASH] valkey primary crashed

7 participants