linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] tools/bootconfig: fix resource leak in apply_xbc()
@ 2020-05-08 10:52 Yunfeng Ye
  2020-05-08 11:03 ` Markus Elfring
  0 siblings, 1 reply; 10+ messages in thread
From: Yunfeng Ye @ 2020-05-08 10:52 UTC (permalink / raw)
  To: mhiramat, rostedt, linux-kernel, kernel-janitors, Markus.Elfring,
	dan.carpenter, Shiyuan Hu, Hewenliang

Fix the @data and @fd allocations that are leaked in the error path of
apply_xbc().

Fixes: 85c46b78da58 ("bootconfig: Add bootconfig magic word for indicating bootconfig explicitly")
Fixes: 950313ebf79c ("tools: bootconfig: Add bootconfig command")
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Yunfeng Ye <yeyunfeng@huawei.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
v2 -> v3:
 - set 'ret' to 0 before returning on success

v1 -> v2:
 - complete the error handling at other error path
 - add "Fixes" tag

 tools/bootconfig/main.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/tools/bootconfig/main.c b/tools/bootconfig/main.c
index 16b9a420e6fd..17a9837dcfaa 100644
--- a/tools/bootconfig/main.c
+++ b/tools/bootconfig/main.c
@@ -314,31 +314,35 @@ int apply_xbc(const char *path, const char *xbc_path)
 	ret = delete_xbc(path);
 	if (ret < 0) {
 		pr_err("Failed to delete previous boot config: %d\n", ret);
-		return ret;
+		goto free_data;
 	}

 	/* Apply new one */
 	fd = open(path, O_RDWR | O_APPEND);
 	if (fd < 0) {
 		pr_err("Failed to open %s: %d\n", path, fd);
-		return fd;
+		ret = fd;
+		goto free_data;
 	}
 	/* TODO: Ensure the @path is initramfs/initrd image */
 	ret = write(fd, data, size + 8);
 	if (ret < 0) {
 		pr_err("Failed to apply a boot config: %d\n", ret);
-		return ret;
+		goto close_fd;
 	}
 	/* Write a magic word of the bootconfig */
 	ret = write(fd, BOOTCONFIG_MAGIC, BOOTCONFIG_MAGIC_LEN);
-	if (ret < 0) {
+	if (ret < 0)
 		pr_err("Failed to apply a boot config magic: %d\n", ret);
-		return ret;
-	}
+
+	ret = 0;
+
+close_fd:
 	close(fd);
+free_data:
 	free(data);

-	return 0;
+	return ret;
 }

 int usage(void)
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] tools/bootconfig: fix resource leak in apply_xbc()
  2020-05-08 10:52 [PATCH v3] tools/bootconfig: fix resource leak in apply_xbc() Yunfeng Ye
@ 2020-05-08 11:03 ` Markus Elfring
  2020-05-08 11:18   ` Yunfeng Ye
  0 siblings, 1 reply; 10+ messages in thread
From: Markus Elfring @ 2020-05-08 11:03 UTC (permalink / raw)
  To: Yunfeng Ye, Steven Rostedt, Masami Hiramatsu
  Cc: linux-kernel, kernel-janitors, Shiyuan Hu, Hewenliang, Dan Carpenter

…
> +++ b/tools/bootconfig/main.c
> @@ -314,31 +314,35 @@ int apply_xbc(const char *path, const char *xbc_path)
> +free_data:
>  	free(data);
…

Would any software users prefer to omit the memory release for
a quicker program termination?

Can the commit message become clearer about the explanation for
the importance of the proposed change?

Regards,
Markus

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] tools/bootconfig: fix resource leak in apply_xbc()
  2020-05-08 11:03 ` Markus Elfring
@ 2020-05-08 11:18   ` Yunfeng Ye
  2020-05-08 11:30     ` [v3] " Markus Elfring
  0 siblings, 1 reply; 10+ messages in thread
From: Yunfeng Ye @ 2020-05-08 11:18 UTC (permalink / raw)
  To: Markus Elfring, Steven Rostedt, Masami Hiramatsu
  Cc: linux-kernel, kernel-janitors, Shiyuan Hu, Hewenliang, Dan Carpenter



On 2020/5/8 19:03, Markus Elfring wrote:
> …
>> +++ b/tools/bootconfig/main.c
>> @@ -314,31 +314,35 @@ int apply_xbc(const char *path, const char *xbc_path)
> …
>> +free_data:
>>  	free(data);
> …
> 
> Would any software users prefer to omit the memory release for
> a quicker program termination?
> 
> Can the commit message become clearer about the explanation for
> the importance of the proposed change?
> 
ok, thanks. this change can fix the warning of tools.

> Regards,
> Markus
> 
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [v3] tools/bootconfig: fix resource leak in apply_xbc()
  2020-05-08 11:18   ` Yunfeng Ye
@ 2020-05-08 11:30     ` Markus Elfring
  2020-05-08 11:42       ` Yunfeng Ye
  0 siblings, 1 reply; 10+ messages in thread
From: Markus Elfring @ 2020-05-08 11:30 UTC (permalink / raw)
  To: Yunfeng Ye, Steven Rostedt, Masami Hiramatsu
  Cc: linux-kernel, kernel-janitors, Shiyuan Hu, Hewenliang, Dan Carpenter

> this change can fix the warning of tools.

Would you like to point any specific source code analysis tools out
for this issue?
(Can a corresponding attribution become relevant for a clearer
change description?)

Regards,
Markus

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [v3] tools/bootconfig: fix resource leak in apply_xbc()
  2020-05-08 11:30     ` [v3] " Markus Elfring
@ 2020-05-08 11:42       ` Yunfeng Ye
  2020-05-08 13:00         ` Steven Rostedt
  0 siblings, 1 reply; 10+ messages in thread
From: Yunfeng Ye @ 2020-05-08 11:42 UTC (permalink / raw)
  To: Markus Elfring, Steven Rostedt, Masami Hiramatsu
  Cc: linux-kernel, kernel-janitors, Shiyuan Hu, Hewenliang, Dan Carpenter



On 2020/5/8 19:30, Markus Elfring wrote:
>> this change can fix the warning of tools.
> 
> Would you like to point any specific source code analysis tools out
> for this issue?
> (Can a corresponding attribution become relevant for a clearer
> change description?)
> 
The tools we used is not for open source. it point out some error description like
"Memory leak: data" and "Resource leak: fd" in tools/bootconfig/main.c.

Can I only description:
  "Memory and resource leak is found by a static code analysis tools" ? thanks.


> Regards,
> Markus
> 
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [v3] tools/bootconfig: fix resource leak in apply_xbc()
  2020-05-08 11:42       ` Yunfeng Ye
@ 2020-05-08 13:00         ` Steven Rostedt
  2020-05-08 13:06           ` Markus Elfring
  2020-05-08 14:45           ` Dan Carpenter
  0 siblings, 2 replies; 10+ messages in thread
From: Steven Rostedt @ 2020-05-08 13:00 UTC (permalink / raw)
  To: Yunfeng Ye
  Cc: Markus Elfring, Masami Hiramatsu, linux-kernel, kernel-janitors,
	Shiyuan Hu, Hewenliang, Dan Carpenter

On Fri, 8 May 2020 19:42:56 +0800
Yunfeng Ye <yeyunfeng@huawei.com> wrote:

> On 2020/5/8 19:30, Markus Elfring wrote:
> >> this change can fix the warning of tools.  
> > 
> > Would you like to point any specific source code analysis tools out
> > for this issue?
> > (Can a corresponding attribution become relevant for a clearer
> > change description?)
> >   
> The tools we used is not for open source. it point out some error description like
> "Memory leak: data" and "Resource leak: fd" in tools/bootconfig/main.c.
> 
> Can I only description:
>   "Memory and resource leak is found by a static code analysis tools" ? thanks.

Markus please stop! Your suggestions are just your preferences that are not
required for the kernel.

Yunfeng, your v2 was fine and has already landed in Linus's tree. Feel free
to ignore Markus's suggestions in the future.

See commit 8842604446d1f005abcbf8c63c12eabdb5695094

-- Steve

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [v3] tools/bootconfig: fix resource leak in apply_xbc()
  2020-05-08 13:00         ` Steven Rostedt
@ 2020-05-08 13:06           ` Markus Elfring
  2020-05-08 14:45           ` Dan Carpenter
  1 sibling, 0 replies; 10+ messages in thread
From: Markus Elfring @ 2020-05-08 13:06 UTC (permalink / raw)
  To: Steven Rostedt, Yunfeng Ye, Masami Hiramatsu
  Cc: linux-kernel, kernel-janitors, Shiyuan Hu, Hewenliang, Dan Carpenter

> Markus please stop! Your suggestions are just your preferences that are not
> required for the kernel.

Do our imaginations and expectations differ for a good commit message?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=79dede78c0573618e3137d3d8cbf78c84e25fabd#n102

Regards,
Markus

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [v3] tools/bootconfig: fix resource leak in apply_xbc()
  2020-05-08 13:00         ` Steven Rostedt
  2020-05-08 13:06           ` Markus Elfring
@ 2020-05-08 14:45           ` Dan Carpenter
  2020-05-08 15:14             ` Steven Rostedt
  1 sibling, 1 reply; 10+ messages in thread
From: Dan Carpenter @ 2020-05-08 14:45 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Yunfeng Ye, Markus Elfring, Masami Hiramatsu, linux-kernel,
	kernel-janitors, Shiyuan Hu, Hewenliang

On Fri, May 08, 2020 at 09:00:53AM -0400, Steven Rostedt wrote:
> On Fri, 8 May 2020 19:42:56 +0800
> Yunfeng Ye <yeyunfeng@huawei.com> wrote:
> 
> > On 2020/5/8 19:30, Markus Elfring wrote:
> > >> this change can fix the warning of tools.  
> > > 
> > > Would you like to point any specific source code analysis tools out
> > > for this issue?
> > > (Can a corresponding attribution become relevant for a clearer
> > > change description?)
> > >   
> > The tools we used is not for open source. it point out some error description like
> > "Memory leak: data" and "Resource leak: fd" in tools/bootconfig/main.c.
> > 
> > Can I only description:
> >   "Memory and resource leak is found by a static code analysis tools" ? thanks.
> 
> Markus please stop! Your suggestions are just your preferences that are not
> required for the kernel.
> 
> Yunfeng, your v2 was fine and has already landed in Linus's tree. Feel free
> to ignore Markus's suggestions in the future.

There was actually a bug in v2.  It exits with a non-zero instead of
zero on success so it will mess up people's scripts.

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [v3] tools/bootconfig: fix resource leak in apply_xbc()
  2020-05-08 14:45           ` Dan Carpenter
@ 2020-05-08 15:14             ` Steven Rostedt
  2020-05-08 16:42               ` Markus Elfring
  0 siblings, 1 reply; 10+ messages in thread
From: Steven Rostedt @ 2020-05-08 15:14 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Yunfeng Ye, Markus Elfring, Masami Hiramatsu, linux-kernel,
	kernel-janitors, Shiyuan Hu, Hewenliang

On Fri, 8 May 2020 17:45:27 +0300
Dan Carpenter <dan.carpenter@oracle.com> wrote:

> > Yunfeng, your v2 was fine and has already landed in Linus's tree. Feel free
> > to ignore Markus's suggestions in the future.  
> 
> There was actually a bug in v2.  It exits with a non-zero instead of
> zero on success so it will mess up people's scripts.

Thanks, I just fixed it. And because of the distraction of Markus's
bikesheding, it was missed :-(

-- Steve

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [v3] tools/bootconfig: fix resource leak in apply_xbc()
  2020-05-08 15:14             ` Steven Rostedt
@ 2020-05-08 16:42               ` Markus Elfring
  0 siblings, 0 replies; 10+ messages in thread
From: Markus Elfring @ 2020-05-08 16:42 UTC (permalink / raw)
  To: Steven Rostedt, Dan Carpenter, Yunfeng Ye, Masami Hiramatsu,
	kernel-janitors
  Cc: linux-kernel, Shiyuan Hu, Hewenliang

>> There was actually a bug in v2.  It exits with a non-zero instead of
>> zero on success so it will mess up people's scripts.
>
> Thanks, I just fixed it. And because of the distraction of Markus's
> bikesheding, it was missed :-(

I am curious how the software development attention will evolve further.
tools/bootconfig: Fix apply_xbc() to return zero on success
https://lore.kernel.org/lkml/20200508111349.3b599bde@gandalf.local.home/
https://lore.kernel.org/patchwork/patch/1239092/
https://lkml.org/lkml/2020/5/8/1342

Will additional change possibilities be taken into account?

Regards,
Markus

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-05-08 16:43 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-08 10:52 [PATCH v3] tools/bootconfig: fix resource leak in apply_xbc() Yunfeng Ye
2020-05-08 11:03 ` Markus Elfring
2020-05-08 11:18   ` Yunfeng Ye
2020-05-08 11:30     ` [v3] " Markus Elfring
2020-05-08 11:42       ` Yunfeng Ye
2020-05-08 13:00         ` Steven Rostedt
2020-05-08 13:06           ` Markus Elfring
2020-05-08 14:45           ` Dan Carpenter
2020-05-08 15:14             ` Steven Rostedt
2020-05-08 16:42               ` Markus Elfring

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).