Bug 29256 - A Potential Memory Leak Bug
Summary: A Potential Memory Leak Bug
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: gas (show other bugs)
Version: 2.34
: P2 normal
Target Milestone: 2.39
Assignee: Alan Modra
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-06-16 12:31 UTC by yuxuan He
Modified: 2022-06-20 02:10 UTC (History)
0 users

See Also:
Host:
Target:
Build:
Last reconfirmed: 2022-06-17 00:00:00


Attachments
diagram of memory leak bug (191.52 KB, image/png)
2022-06-16 12:31 UTC, yuxuan He
Details

Note You need to log in before you can comment on or make changes to this bug.
Description yuxuan He 2022-06-16 12:31:08 UTC
Created attachment 14148 [details]
diagram of memory leak bug

Hi, I found a potential memory leak bug in the project source code of binutils, I have shown the execution sequence of the program that may generate the bug on a diagram which is added to the attachment.
The text in red illustrates the steps that generate the bug
The red arrows represent call relationships
The green text illustrates the files and functions whose code snippets are located below the green text.
The purple text is the annotation of the step that caused the bug

In short, the key to the bug is the fifth step in the diagram. The return value of the function called in step 5 points to dynamically allocated memory, and after that call, the return value of the function obj_elf_section_name is not passed further to the caller of function obj_elf_section, and no free operation is performed, thus a memory leak occurs.
It is worth noting that although the second and third steps in the diagram show that name is a dynamically allocated memory, other defined sites of name are also dynamically allocated memory, for example, line 975 of the code fragment where the second step in the diagram is located calls the function xmemdup0, which also uses xmalloc to dynamically allocate memory and passing the return value to name


I look forward to your reply and thank you very much for your patience
Comment 1 cvs-commit@gcc.gnu.org 2022-06-17 11:59:37 UTC
The master branch has been updated by Alan Modra <amodra@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=15797439805b1bb92c491b9c9a72500d93c0cb5b

commit 15797439805b1bb92c491b9c9a72500d93c0cb5b
Author: Alan Modra <amodra@gmail.com>
Date:   Fri Jun 17 17:25:53 2022 +0930

    PR29256, memory leak in obj_elf_section_name
    
    When handling section names in quotes obj_elf_section_name calls
    demand_copy_C_string, which puts the name on the gas notes obstack.
    Such strings aren't usually freed, since obstack_free frees all more
    recently allocated objects as well as its arg.  When handling
    non-quoted names, obj_elf_section_name mallocs the name.  Due to the
    mix of allocation strategies it isn't possible for callers to free
    names, if that was desirable.  Partially fix this by always creating
    names on the obstack, which is more efficient anyway.  (You still
    can't obstack_free on error paths due to the xtensa
    tc_canonicalize_section_name.)  Also remove a couple of cases where
    the name is dup'd for no good reason as far as I know.
    
            PR 29256
            * config/obj-elf.c (obj_elf_section_name): Create name on notes
            obstack.
            (obj_elf_attach_to_group): Don't strdup group name.
            (obj_elf_section): Likewise.
            (obj_elf_vendor_attribute): Use xmemdup0 rather than xstrndup.
Comment 2 yuxuan He 2022-06-17 12:25:48 UTC
(In reply to cvs-commit@gcc.gnu.org from comment #1)
> The master branch has been updated by Alan Modra <amodra@sourceware.org>:
> 
> https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;
> h=15797439805b1bb92c491b9c9a72500d93c0cb5b
> 
> commit 15797439805b1bb92c491b9c9a72500d93c0cb5b
> Author: Alan Modra <amodra@gmail.com>
> Date:   Fri Jun 17 17:25:53 2022 +0930
> 
>     PR29256, memory leak in obj_elf_section_name
>     
>     When handling section names in quotes obj_elf_section_name calls
>     demand_copy_C_string, which puts the name on the gas notes obstack.
>     Such strings aren't usually freed, since obstack_free frees all more
>     recently allocated objects as well as its arg.  When handling
>     non-quoted names, obj_elf_section_name mallocs the name.  Due to the
>     mix of allocation strategies it isn't possible for callers to free
>     names, if that was desirable.  Partially fix this by always creating
>     names on the obstack, which is more efficient anyway.  (You still
>     can't obstack_free on error paths due to the xtensa
>     tc_canonicalize_section_name.)  Also remove a couple of cases where
>     the name is dup'd for no good reason as far as I know.
>     
>             PR 29256
>             * config/obj-elf.c (obj_elf_section_name): Create name on notes
>             obstack.
>             (obj_elf_attach_to_group): Don't strdup group name.
>             (obj_elf_section): Likewise.
>             (obj_elf_vendor_attribute): Use xmemdup0 rather than xstrndup.

thank you!
Comment 3 Alan Modra 2022-06-20 02:10:40 UTC
I think this is the best we can do here, closing.