2024-03-23 08:46:12

by lumingyindetect

[permalink] [raw]
Subject: [PATCH] tools:Fix a memory leak related to variable name

In the elf_create_prefix_symbol function defined in the /linux/tools/objtool/elf.c file, two pointer variables sym and name are defined. The program allocates dynamic memory for the pointer sym using the calloc function at line 822, and for the pointer name using the malloc function at line 824. When the if statement at line 826 returns true, the program returns at line 828. The content of the if statement at line 828 is if (sym==NULL || name==NULL), which checks if either sym or name is NULL. If this condition returns true, it indicates a situation where one of the pointers has successfully allocated memory but the other has not. Therefore, if the if statement returns true, directly returning may lead to memory leak issues. Hence, in the code, I have added checks separately for whether sym and name are NULL, and if they are not NULL, the corresponding dynamic memory spaces are freed.

Signed-off-by: LuMingYin <[email protected]>
---
tools/objtool/elf.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index 3d27983dc908..42d20cd08936 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -825,6 +825,12 @@ elf_create_prefix_symbol(struct elf *elf, struct symbol *orig, long size)

if (!sym || !name) {
perror("malloc");
+ if(sym){
+ free(sym);
+ }
+ if(name){
+ free(name);
+ }
return NULL;
}

--
2.25.1



2024-03-23 18:22:17

by Josh Poimboeuf

[permalink] [raw]
Subject: Re: [PATCH] tools:Fix a memory leak related to variable name

On Sat, Mar 23, 2024 at 04:45:26PM +0800, LuMingYin wrote:
> In the elf_create_prefix_symbol function defined in the /linux/tools/objtool/elf.c file, two pointer variables sym and name are defined. The program allocates dynamic memory for the pointer sym using the calloc function at line 822, and for the pointer name using the malloc function at line 824. When the if statement at line 826 returns true, the program returns at line 828. The content of the if statement at line 828 is if (sym==NULL || name==NULL), which checks if either sym or name is NULL. If this condition returns true, it indicates a situation where one of the pointers has successfully allocated memory but the other has not. Therefore, if the if statement returns true, directly returning may lead to memory leak issues. Hence, in the code, I have added checks separately for whether sym and name are NULL, and if they are not NULL, the corresponding dynamic memory spaces are freed.
>
> Signed-off-by: LuMingYin <[email protected]>

Thanks for the patch. In general we don't care about memory leaks in
objtool (particularly in error or exit paths), as it's a short-running
tool. When it exits, all the memory will be freed anyway. So it's
faster to not free the memory manually.

--
Josh

2024-03-24 05:57:08

by lumingyindetect

[permalink] [raw]
Subject: Re:Re: [PATCH] tools:Fix a memory leak related to variable name

Thank&nbsp;you&nbsp;for&nbsp;your&nbsp;prompt&nbsp;response!&nbsp;It&nbsp;is&nbsp;indeed&nbsp;a&nbsp;wise&nbsp;decision&nbsp;not&nbsp;to&nbsp;release&nbsp;dynamic&nbsp;memory&nbsp;when&nbsp;the&nbsp;tool&nbsp;runs&nbsp;briefly&nbsp;and&nbsp;encounters&nbsp;errors.&nbsp;However,&nbsp;I&nbsp;also&nbsp;noticed&nbsp;in&nbsp;the&nbsp;disas_funcs&nbsp;function&nbsp;in&nbsp;the&nbsp;/linux/tools/objtool/check.c&nbsp;file&nbsp;(line&nbsp;4617)&nbsp;that&nbsp;a&nbsp;variable&nbsp;named&nbsp;cmd&nbsp;pointing&nbsp;to&nbsp;a&nbsp;dynamic&nbsp;memory&nbsp;area&nbsp;is&nbsp;not&nbsp;being&nbsp;freed&nbsp;(regardless&nbsp;of&nbsp;whether&nbsp;an&nbsp;error&nbsp;occurs).&nbsp;In&nbsp;this&nbsp;case,&nbsp;would&nbsp;it&nbsp;be&nbsp;necessary&nbsp;to&nbsp;add&nbsp;a&nbsp;free(cmd)?
在 2024-03-24 02:22:09,"Josh Poimboeuf" <[email protected]> 写道:
On Sat, Mar 23, 2024 at 04:45:26PM +0800, LuMingYin wrote:
> In the elf_create_prefix_symbol function defined in the /linux/tools/objtool/elf.c file, two pointer variables sym and name are defined. The program allocates dynamic memory for the pointer sym using the calloc function at line 822, and for the pointer name using the malloc function at line 824. When the if statement at line 826 returns true, the program returns at line 828. The content of the if statement at line 828 is if (sym==NULL || name==NULL), which checks if either sym or name is NULL. If this condition returns true, it indicates a situation where one of the pointers has successfully allocated memory but the other has not. Therefore, if the if statement returns true, directly returning may lead to memory leak issues. Hence, in the code, I have added checks separately for whether sym and name are NULL, and if they are not NULL, the corresponding dynamic memory spaces are freed.
>
> Signed-off-by: LuMingYin <[email protected]>

Thanks for the patch. In general we don't care about memory leaks in
objtool (particularly in error or exit paths), as it's a short-running
tool. When it exits, all the memory will be freed anyway. So it's
faster to not free the memory manually.

--
Josh

2024-03-24 06:00:00

by lumingyindetect

[permalink] [raw]
Subject: Re:Re: [PATCH] tools:Fix a memory leak related to variable name

Thank you for your prompt response! It is indeed a wise decision not to release dynamic memory when the tool runs briefly and encounters errors. However, I also noticed in the disas_funcs function in the /linux/tools/objtool/check.c file (line 4617) that a variable named cmd pointing to a dynamic memory area is not being freed (regardless of whether an error occurs). In this case, would it be necessary to add a free(cmd)?<br/><br/>--<br/>LuMingYin
At 2024-03-24 02:22:09, "Josh Poimboeuf" <[email protected]> wrote:
>On Sat, Mar 23, 2024 at 04:45:26PM +0800, LuMingYin wrote:
>> In the elf_create_prefix_symbol function defined in the /linux/tools/objtool/elf.c file, two pointer variables sym and name are defined. The program allocates dynamic memory for the pointer sym using the calloc function at line 822, and for the pointer name using the malloc function at line 824. When the if statement at line 826 returns true, the program returns at line 828. The content of the if statement at line 828 is if (sym==NULL || name==NULL), which checks if either sym or name is NULL. If this condition returns true, it indicates a situation where one of the pointers has successfully allocated memory but the other has not. Therefore, if the if statement returns true, directly returning may lead to memory leak issues. Hence, in the code, I have added checks separately for whether sym and name are NULL, and if they are not NULL, the corresponding dynamic memory spaces are freed.
>>
>> Signed-off-by: LuMingYin <[email protected]>
>
>Thanks for the patch. In general we don't care about memory leaks in
>objtool (particularly in error or exit paths), as it's a short-running
>tool. When it exits, all the memory will be freed anyway. So it's
>faster to not free the memory manually.
>
>--
>Josh