linux-kernel - Re: [PATCH v2 1/1] ftrace: do not update max buffer with no users

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20081113140102.GA14258@elte.hu>
Date:	Thu, 13 Nov 2008 15:01:02 +0100
From:	Ingo Molnar <mingo@...e.hu>
To:	Steven Rostedt <rostedt@...dmis.org>
Cc:	linux-kernel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Pekka Paalanen <pq@....fi>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Steven Rostedt <srostedt@...hat.com>
Subject: Re: [PATCH v2 1/1] ftrace: do not update max buffer with no users


* Steven Rostedt <rostedt@...dmis.org> wrote:

> 
> On Thu, 13 Nov 2008, Ingo Molnar wrote:
> > 
> > the obvious solution is to add this to ring_buffer_resize():
> > 
> > 	if (!buffer)
> > 		return size;
> 
> Having a NULL buffer return a successful resize is a bit worrisome to me.
> 
> And looking at the code I was trying to make sure could never be called
> if there are no max_tr users:
> 
> in update_max_tr
> 
> 	buf = tr->buffer;
> 	tr->buffer = max_tr.buffer;
> 	max_tr.buffer = buf;
> 
> Should all the ring buffer API return success on NULL pointers?

well, update_max_tr() is only used from tracers which also actually 
allocate a max trace buffer: irqs/preemptoff, sched-wakeup.

So, can you tell me any way how the patch below could be broken?

	Ingo

------------->
>From ee51a1de7e3837577412be269e0100038068e691 Mon Sep 17 00:00:00 2001
From: Ingo Molnar <mingo@...e.hu>
Date: Thu, 13 Nov 2008 14:58:31 +0100
Subject: [PATCH] tracing: fix mmiotrace resizing crash

Pekka reported a crash when resizing the mmiotrace tracer (if only
mmiotrace is enabled).

This happens because in that case we do not allocate the max buffer,
but we try to use it.

Make ring_buffer_resize() idempotent against NULL buffers.

Reported-by: Pekka Paalanen <pq@....fi>
Signed-off-by: Ingo Molnar <mingo@...e.hu>
---
 kernel/trace/ring_buffer.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 231db20..036456c 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -538,6 +538,12 @@ int ring_buffer_resize(struct ring_buffer *buffer, unsigned long size)
 	LIST_HEAD(pages);
 	int i, cpu;
 
+	/*
+	 * Always succeed at resizing a non-existent buffer:
+	 */
+	if (!buffer)
+		return size;
+
 	size = DIV_ROUND_UP(size, BUF_PAGE_SIZE);
 	size *= BUF_PAGE_SIZE;
 	buffer_size = buffer->pages * BUF_PAGE_SIZE;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/