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

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

[zhangyi (F)]

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) <yi.zhang@huawei.com>
---
 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

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

[Steven Rostedt]

On Wed, 13 Feb 2019 20:29:06 +0800
"zhangyi (F)" <yi.zhang@huawei.com> 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) <yi.zhang@huawei.com>
> ---
>  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);

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

[zhangyi (F)]

On 2019/2/13 21:36, Steven Rostedt Wrote:
> On Wed, 13 Feb 2019 20:29:06 +0800
> "zhangyi (F)" <yi.zhang@huawei.com> 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) <yi.zhang@huawei.com>
>> ---
>>  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);
> 
> 
> .
> 

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

[Steven Rostedt]

On Sat, 16 Feb 2019 19:19:03 +0800
"zhangyi (F)" <yi.zhang@huawei.com> wrote:

> On 2019/2/13 21:36, Steven Rostedt Wrote:
> > On Wed, 13 Feb 2019 20:29:06 +0800
> > "zhangyi (F)" <yi.zhang@huawei.com> 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