Re: [PATCH 1/2] Fix memory leak in __sbitmap_for_each_set function

[lijiang <lijiang@xxxxxxxxxx>]

Thank you for the fix, Sergey.

On Wed, Mar 9, 2022 at 8:00 PM <crash-utility-request@xxxxxxxxxx> wrote:

Date: Tue, 8 Mar 2022 23:27:09 +0300
From: Sergey Samoylenko <s.samoylenko@xxxxxxxxx>
To: <crash-utility@xxxxxxxxxx>
Cc: <linux@xxxxxxxxx>, Sergey Samoylenko <s.samoylenko@xxxxxxxxx>
Subject: [PATCH 1/2] Fix memory leak in
        __sbitmap_for_each_set function
Message-ID: <20220308202710.46668-2-s.samoylenko@xxxxxxxxx>
Content-Type: text/plain

Signed-off-by: Sergey Samoylenko <s.samoylenko@xxxxxxxxx>
---
 sbitmap.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sbitmap.c b/sbitmap.c
index 91a5274..4eaa0cc 100644
--- a/sbitmap.c
+++ b/sbitmap.c

This reminds me, if parse_line() or dump_struct_member() fails, is there a potential risk of memory leaks in the dump_struct_members()?

 

file: sbitmap.c

432 static void dump_struct_members(const char *s, ulong addr, unsigned radix)
433 {
434         int i, argc;
435         char *p1, *p2;
436         char *structname, *members;
437         char *arglist[MAXARGS];
438
439         structname = GETBUF(strlen(s) + 1);
440         members = GETBUF(strlen(s) + 1);
441
442         strcpy(structname, s);
443         p1 = strstr(structname, ".") + 1;
444
445         p2 = strstr(s, ".") + 1;
446         strcpy(members, p2);
447         replace_string(members, ",", ' ');
448         argc = parse_line(members, arglist);
449
450         for (i = 0; i < argc; i++) {
451                 *p1 = NULLCHAR;
452                 strcat(structname, arglist[i]);
453                 dump_struct_member(structname, addr, radix);
454         }
455
456         FREEBUF(structname);
457         FREEBUF(members);
458 }

I noticed that the parse_line() has a return value, but the dump_struct_member() has no return value, is there a good way to avoid the potential risks? Or leave it there?

BTW: I saw the similar issues in tools.c

Thanks.

Lianbo

@@ -272,7 +272,7 @@ static void __sbitmap_for_each_set(const struct sbitmap_context *sc,
                        if (nr >= depth)
                                break;
                        if (!fn((index << sc->shift) + nr, data))
-                               return;
+                               goto exit;

                        nr++;
                }
@@ -282,6 +282,7 @@ next:
                        index = 0;
        }

+exit:
        FREEBUF(sbitmap_word_buf);
 }

--
2.25.1

--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility
Contribution Guidelines: https://github.com/crash-utility/crash/wiki