Skip to content

Conversation

sundb
Copy link
Collaborator

@sundb sundb commented Sep 4, 2025

This PR fixes two defrag issues.

  1. Fix a use-after-free issue caused by updating dictionary keys after a pubsub channel is reallocated.
    This issue was introduced by Implement defragmentation for pubsub kvstore #13058

  2. Fix potential use-after-free for lua during AOF loading with defrag
    This issue was introduced by Implement defragmentation for pubsub kvstore #13058
    This fix follows Fix crash during lua script defrag #14319
    This PR updates the LuaScript LRU list before script execution to prevent accessing a potentially invalidated pointer after long-running scripts.

Note that both of these issues were introduced since 7.4, so we can fix them together.

Copy link

snyk-io bot commented Sep 4, 2025

🎉 Snyk checks have passed. No issues have been found so far.

security/snyk check is complete. No issues have been found. (View Details)

license/snyk check is complete. No issues have been found. (View Details)

@sundb sundb added the release-notes indication that this issue needs to be mentioned in the release notes label Sep 4, 2025
@github-project-automation github-project-automation bot moved this to Todo in Redis 8.4 Sep 4, 2025
@sundb sundb changed the title Fix crash during pubsub defrag Fix crash during pubsub defrag and lua defrag Sep 4, 2025
@sundb sundb changed the title Fix crash during pubsub defrag and lua defrag Fix defrag issues for pubsub and lua Sep 4, 2025
@sundb sundb requested a review from Copilot September 4, 2025 06:21
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes defragmentation issues for pubsub and lua functionality that were introduced by a previous change. The fix addresses problems where memory defragmentation could reallocate objects that were still being referenced, leading to potential use-after-free scenarios.

  • Moved lua script execution to occur after LRU list manipulation to prevent accessing reallocated memory
  • Added a new dictionary lookup function that uses pointer comparison instead of key comparison for defrag scenarios
  • Updated pubsub defrag callback to use the new lookup method to handle reallocated channel names

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
src/eval.c Reordered lua script execution and LRU operations, added safety comment about memory reallocation
src/dict.h Added declaration for new dictFindByHashAndPtr function
src/dict.c Implemented dictFindByHashAndPtr function for pointer-based dictionary lookups
src/defrag.c Updated pubsub defrag callback to use hash and pointer lookup instead of key-based lookup

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@sundb sundb merged commit d339fe7 into redis:unstable Sep 7, 2025
19 checks passed
@github-project-automation github-project-automation bot moved this from Todo to Done in Redis 8.4 Sep 7, 2025
YaacovHazan pushed a commit to YaacovHazan/redis that referenced this pull request Sep 28, 2025
This PR fixes two defrag issues.

1. Fix a use-after-free issue caused by updating dictionary keys after a
pubsub channel is reallocated.
This issue was introduced by redis#13058

1. Fix potential use-after-free for lua during AOF loading with defrag
This issue was introduced by redis#13058
   This fix follows redis#14319
This PR updates the LuaScript LRU list before script execution to
prevent accessing a potentially invalidated pointer after long-running
scripts.
sundb added a commit to YaacovHazan/redis that referenced this pull request Sep 30, 2025
This PR fixes two defrag issues.

1. Fix a use-after-free issue caused by updating dictionary keys after a
pubsub channel is reallocated.
This issue was introduced by redis#13058

1. Fix potential use-after-free for lua during AOF loading with defrag
This issue was introduced by redis#13058
   This fix follows redis#14319
This PR updates the LuaScript LRU list before script execution to
prevent accessing a potentially invalidated pointer after long-running
scripts.
sundb added a commit to YaacovHazan/redis that referenced this pull request Sep 30, 2025
This PR fixes two defrag issues.

1. Fix a use-after-free issue caused by updating dictionary keys after a
pubsub channel is reallocated.
This issue was introduced by redis#13058

2. Fix potential use-after-free for lua during AOF loading with defrag
This issue was introduced by redis#13058
   This fix follows redis#14319
This PR updates the LuaScript LRU list before script execution to
prevent accessing a potentially invalidated pointer after long-running
scripts.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-notes indication that this issue needs to be mentioned in the release notes

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants