linux-kernel - Re: [PATCH] tracing: Fix crash when it fails to alloc ring buffer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 26 Dec 2017 11:22:24 -0500
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Chunyan Zhang <chunyan.zhang@...eadtrum.com>
Cc:     Ingo Molnar <mingo@...hat.com>, <linux-kernel@...r.kernel.org>,
        Jing Xia <jing.xia@...eadtrum.com>,
        Orson Zhai <orson.zhai@...eadtrum.com>,
        Chunyan Zhang <zhang.lyra@...il.com>
Subject: Re: [PATCH] tracing: Fix crash when it fails to alloc ring buffer

On Tue, 26 Dec 2017 15:12:53 +0800
Chunyan Zhang <chunyan.zhang@...eadtrum.com> wrote:

> From: Jing Xia <jing.xia@...eadtrum.com>
> 
> Double free of the ring buffer happens when it fails to alloc new
> ring buffer instance for max_buffer if TRACER_MAX_TRACE is configured.
> The root cause is that the pointer is not set to NULL after the buffer
> is freed in allocate_trace_buffers(), and the freeing of the ring
> buffer is invoked again later if the pointer is not equal to Null,
> as:
> 
> instance_mkdir()
>     |-allocate_trace_buffers()
>         |-allocate_trace_buffer(tr, &tr->trace_buffer...)
> 	|-allocate_trace_buffer(tr, &tr->max_buffer...)
> 
>           // allocate fail(-ENOMEM),first free
>           // and the buffer pointer is not set to null
>         |-ring_buffer_free(tr->trace_buffer.buffer)
> 
>        // out_free_tr
>     |-free_trace_buffers()
>         |-free_trace_buffer(&tr->trace_buffer);
> 
> 	      //if trace_buffer is not null, free again
> 	    |-ring_buffer_free(buf->buffer)
>                 |-rb_free_cpu_buffer(buffer->buffers[cpu])
>                     // ring_buffer_per_cpu is null, and
>                     // crash in ring_buffer_per_cpu->pages
> 
> Signed-off-by: Jing Xia <jing.xia@...eadtrum.com>
> Signed-off-by: Chunyan Zhang <chunyan.zhang@...eadtrum.com>

Thanks!

I'll apply this to my tree and start testing it.

-- Steve

> ---
>  kernel/trace/trace.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 73e67b6..ed1c0d1 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -7622,7 +7622,9 @@ static int allocate_trace_buffers(struct trace_array *tr, int size)
>  				    allocate_snapshot ? size : 1);
>  	if (WARN_ON(ret)) {
>  		ring_buffer_free(tr->trace_buffer.buffer);
> +		tr->trace_buffer.buffer = NULL;
>  		free_percpu(tr->trace_buffer.data);
> +		tr->trace_buffer.data = NULL;
>  		return -ENOMEM;
>  	}
>  	tr->allocated_snapshot = allocate_snapshot;