Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] There is a memory leak defect at line 5981 of the file redis-cli.c in /redis/src/. #13256

Open
LuMingYinDetect opened this issue May 10, 2024 · 4 comments

Comments

@LuMingYinDetect
Copy link

Describe the bug

A variable named "slot_nodes" is defined at line 5967 of the file redis-cli.c in /redis/src/. It allocates a block of dynamic memory using the listCreate function. When the if statement at line 5978 evaluates to true, the program jumps to the "cleanup" label at line 5981 using a goto statement. During this process, the dynamic memory allocated to slot_nodes is neither used as shown in line 5994 nor released at the "cleanup" label, resulting in a memory leak defect.as shown in the figure below:
https://github.com/LuMingYinDetect/redis_defects/blob/main/redis_1.png

To reproduce

The detection tool I'm using is the Clang Static Analyzer, which employs static analysis techniques. The tool's defect reports provide the path that triggers the defect. Based on the aforementioned path, the defect can be reproduced.

Expected behavior

If the defect is confirmed, it is advisable to address it by making necessary fixes.

@sundb
Copy link
Collaborator

sundb commented May 10, 2024

@LuMingYinDetect Thanks, do you wanna make a PR for this?

@LuMingYinDetect
Copy link
Author

@LuMingYinDetect Thanks, do you wanna make a PR for this?

Thank you for your prompt response! I'm at a loss on how to fix this defect. Could I trouble you to fix it?

@sundb
Copy link
Collaborator

sundb commented May 10, 2024

@LuMingYinDetect a minor patch, if you want welcome to make PR for it, also i can do it if you need me.

diff --git a/src/redis-cli.c b/src/redis-cli.c
index 0c9f088da..e8484956c 100644
--- a/src/redis-cli.c
+++ b/src/redis-cli.c
@@ -5978,6 +5978,7 @@ static int clusterManagerFixSlotsCoverage(char *all_slots) {
                 if (!clusterManagerCheckRedisReply(n, reply, NULL)) {
                     fixed = -1;
                     if (reply) freeReplyObject(reply);
+                    if (slot_nodes) listRelease(slot_nodes);
                     goto cleanup;
                 }
                 assert(reply->type == REDIS_REPLY_ARRAY);

LuMingYinDetect added a commit to LuMingYinDetect/redis that referenced this issue May 11, 2024
@LuMingYinDetect
Copy link
Author

@LuMingYinDetect a minor patch, if you want welcome to make PR for it, also i can do it if you need me.

diff --git a/src/redis-cli.c b/src/redis-cli.c
index 0c9f088da..e8484956c 100644
--- a/src/redis-cli.c
+++ b/src/redis-cli.c
@@ -5978,6 +5978,7 @@ static int clusterManagerFixSlotsCoverage(char *all_slots) {
                 if (!clusterManagerCheckRedisReply(n, reply, NULL)) {
                     fixed = -1;
                     if (reply) freeReplyObject(reply);
+                    if (slot_nodes) listRelease(slot_nodes);
                     goto cleanup;
                 }
                 assert(reply->type == REDIS_REPLY_ARRAY);

Thank you for your patient explanation! I have submitted a pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants