Re: [PATCH v3] tracing/probes: Fix memory leak in traceprobe_parse_probe_arg_body()

[Google]

Hi LuMing,

On Sat, 27 Apr 2024 08:23:47 +0100
lumingyindet...@126.com wrote:

> From: LuMingYin 
> 
> If traceprobe_parse_probe_arg_body() failed to allocate 'parg->fmt',
> it jumps to the label 'out' instead of 'fail' by mistake.In the result,
> the buffer 'tmp' is not freed in this case and leaks its memory.
> 
> Thus jump to the label 'fail' in that error case.
> 

Looks good to me.

Acked-by: Masami Hiramatsu (Google) 


Thank you!

> Fixes: 032330abd08b ("tracing/probes: Cleanup probe argument parser")
> Signed-off-by: LuMingYin 
> ---
>  kernel/trace/trace_probe.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> index c09fa6fc636e..81c319b92038 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -1466,7 +1466,7 @@ static int traceprobe_parse_probe_arg_body(const char 
> *argv, ssize_t *size,
>   parg->fmt = kmalloc(len, GFP_KERNEL);
>   if (!parg->fmt) {
>   ret = -ENOMEM;
> - goto out;
> + goto fail;
>   }
>   snprintf(parg->fmt, len, "%s[%d]", parg->type->fmttype,
>parg->count);
> -- 
> 2.25.1
> 
> 


-- 
Masami Hiramatsu (Google) 

Re: [PATCH v3] tracing/probes: Fix memory leak in traceprobe_parse_probe_arg_body()

[Markus Elfring]

> … by mistake.In the result,
…

I propose once more to start the second sentence of this change description
on a subsequent line.

> ---
> kernel/trace/trace_probe.c | 2 +-
…

Unfortunately, you overlooked to add patch version descriptions behind the 
marker line.

See also:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.9-rc5#n713

Regards,
Markus

[PATCH v3] tracing/probes: Fix memory leak in traceprobe_parse_probe_arg_body()

[lumingyindetect]

From: LuMingYin 

If traceprobe_parse_probe_arg_body() failed to allocate 'parg->fmt',
it jumps to the label 'out' instead of 'fail' by mistake.In the result,
the buffer 'tmp' is not freed in this case and leaks its memory.

Thus jump to the label 'fail' in that error case.

Fixes: 032330abd08b ("tracing/probes: Cleanup probe argument parser")
Signed-off-by: LuMingYin 
---
 kernel/trace/trace_probe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index c09fa6fc636e..81c319b92038 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -1466,7 +1466,7 @@ static int traceprobe_parse_probe_arg_body(const char 
*argv, ssize_t *size,
parg->fmt = kmalloc(len, GFP_KERNEL);
if (!parg->fmt) {
ret = -ENOMEM;
-   goto out;
+   goto fail;
}
snprintf(parg->fmt, len, "%s[%d]", parg->type->fmttype,
 parg->count);
-- 
2.25.1