2019-02-13 14:07:10

by Zhang Yi

Subject: [PATCH] tracing: fix incorrect tracer freeing when opening tracing pipe

Commit d716ff71dd12 ("tracing: Remove taking of trace_types_lock in
pipe files") use the current tracer instead of the copy in
tracing_open_pipe(), but it forget to remove the freeing sentence in
the error path.

Fixes: d716ff71dd12 ("tracing: Remove taking of trace_types_lock in pipe files")
Signed-off-by: zhangyi (F) <[email protected]>
---
kernel/trace/trace.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index c521b73..b583ff7 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -5624,7 +5624,6 @@ static int tracing_open_pipe(struct inode *inode, struct file *filp)
return ret;

fail:
- kfree(iter->trace);
kfree(iter);
__trace_array_put(tr);
mutex_unlock(&trace_types_lock);
--
2.7.4

2019-02-13 14:54:57

by Steven Rostedt

Subject: Re: [PATCH] tracing: fix incorrect tracer freeing when opening tracing pipe

On Wed, 13 Feb 2019 20:29:06 +0800
"zhangyi (F)" <[email protected]> wrote:

> Commit d716ff71dd12 ("tracing: Remove taking of trace_types_lock in
> pipe files") use the current tracer instead of the copy in
> tracing_open_pipe(), but it forget to remove the freeing sentence in
> the error path.
>
> Fixes: d716ff71dd12 ("tracing: Remove taking of trace_types_lock in pipe files")

Thanks.

As this is harmless (frees to NULL is ok, and iter is allocated with
kzalloc()), I'm going to just add this for the next merge window.

-- Steve

> Signed-off-by: zhangyi (F) <[email protected]>
> ---
> kernel/trace/trace.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index c521b73..b583ff7 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -5624,7 +5624,6 @@ static int tracing_open_pipe(struct inode *inode, struct file *filp)
> return ret;
>
> fail:
> - kfree(iter->trace);
> kfree(iter);
> __trace_array_put(tr);
> mutex_unlock(&trace_types_lock);

2019-02-16 13:07:16

by Zhang Yi

Subject: Re: [PATCH] tracing: fix incorrect tracer freeing when opening tracing pipe

On 2019/2/13 21:36, Steven Rostedt Wrote:
> On Wed, 13 Feb 2019 20:29:06 +0800
> "zhangyi (F)" <[email protected]> wrote:
>
>> Commit d716ff71dd12 ("tracing: Remove taking of trace_types_lock in
>> pipe files") use the current tracer instead of the copy in
>> tracing_open_pipe(), but it forget to remove the freeing sentence in
>> the error path.
>>
>> Fixes: d716ff71dd12 ("tracing: Remove taking of trace_types_lock in pipe files")
>
> Thanks.
>
> As this is harmless (frees to NULL is ok, and iter is allocated with
> kzalloc()), I'm going to just add this for the next merge window.
>

This patch patch want to remove kfree(iter->trace) instead of kfree(iter),
which is set by tr->current_trace and probably not allocated dynamically.
So if we kfree such tracer in the error path, it will lead to BUG_ON.

Thanks，
Yi.

>
>> Signed-off-by: zhangyi (F) <[email protected]>
>> ---
>> kernel/trace/trace.c | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
>> index c521b73..b583ff7 100644
>> --- a/kernel/trace/trace.c
>> +++ b/kernel/trace/trace.c
>> @@ -5624,7 +5624,6 @@ static int tracing_open_pipe(struct inode *inode, struct file *filp)
>> return ret;
>>
>> fail:
>> - kfree(iter->trace);
>> kfree(iter);
>> __trace_array_put(tr);
>> mutex_unlock(&trace_types_lock);
>
>
> .
>

2019-02-20 19:35:48

by Steven Rostedt

Subject: Re: [PATCH] tracing: fix incorrect tracer freeing when opening tracing pipe

On Sat, 16 Feb 2019 19:19:03 +0800
"zhangyi (F)" <[email protected]> wrote:

> On 2019/2/13 21:36, Steven Rostedt Wrote:
> > On Wed, 13 Feb 2019 20:29:06 +0800
> > "zhangyi (F)" <[email protected]> wrote:
> >
> >> Commit d716ff71dd12 ("tracing: Remove taking of trace_types_lock in
> >> pipe files") use the current tracer instead of the copy in
> >> tracing_open_pipe(), but it forget to remove the freeing sentence in
> >> the error path.
> >>
> >> Fixes: d716ff71dd12 ("tracing: Remove taking of trace_types_lock in pipe files")
> >
> > Thanks.
> >
> > As this is harmless (frees to NULL is ok, and iter is allocated with
> > kzalloc()), I'm going to just add this for the next merge window.
> >
>
> This patch patch want to remove kfree(iter->trace) instead of kfree(iter),
> which is set by tr->current_trace and probably not allocated dynamically.
> So if we kfree such tracer in the error path, it will lead to BUG_ON.
>

This should have been mentioned in the change log. Anyway, I tagged it
for stable, but it will be pulled in during the next merge window, as
that shouldn't be too long from now.

-- Steve