All of lore.kernel.org
 help / color / mirror / Atom feed
* perf/ring-buffer: Undefined behaviour in kernel/events/ring_buffer.c:685:22
@ 2016-01-10 20:55 Sasha Levin
  2016-01-19 14:31 ` Peter Zijlstra
  0 siblings, 1 reply; 6+ messages in thread
From: Sasha Levin @ 2016-01-10 20:55 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo; +Cc: LKML

Hi all,

While fuzzing with trinity inside a KVM tools guest, running the latest -next
kernel, I've hit the following warning:

[ 3494.030114] UBSAN: Undefined behaviour in kernel/events/ring_buffer.c:685:22
[ 3494.030647] shift exponent -1 is negative
[ 3494.031025] CPU: 1 PID: 28506 Comm: kworker/1:2 Not tainted 4.4.0-rc8-next-20160108-sasha-00024-gaaecb9a #2780
[ 3494.032861] Workqueue: events rb_free_work
[ 3494.033195]  1ffff10018bb2f1b 0000000025cc19e7 ffff8800c5d97958 ffffffffa101a182
[ 3494.033871]  0000000041b58ab3 ffffffffac1b3838 ffffffffa101a0b7 ffff8800c5d97920
[ 3494.034469]  ffffffffae01db80 ffff8800c5d97940 0000000025cc19e7 ffffffffae01db80
[ 3494.035097] Call Trace:
[ 3494.035336]  [<ffffffffa101a182>] dump_stack+0xcb/0x149
[ 3494.036171]  [<ffffffffa1114296>] ubsan_epilogue+0x12/0x8f
[ 3494.036568]  [<ffffffffa1114e9a>] __ubsan_handle_shift_out_of_bounds+0x2a3/0x308
[ 3494.038670]  [<ffffffff9f6274ee>] rb_free_work+0xae/0x1a0
[ 3494.039491]  [<ffffffff9f3ac74c>] process_one_work+0xb6c/0x13d0
[ 3494.042924]  [<ffffffff9f3adce3>] worker_thread+0xd33/0x1150
[ 3494.043923]  [<ffffffff9f3c4239>] kthread+0x2f9/0x310


Thanks,
Sasha

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: perf/ring-buffer: Undefined behaviour in kernel/events/ring_buffer.c:685:22
  2016-01-10 20:55 perf/ring-buffer: Undefined behaviour in kernel/events/ring_buffer.c:685:22 Sasha Levin
@ 2016-01-19 14:31 ` Peter Zijlstra
  2016-01-21 23:34   ` Sasha Levin
  2016-01-22 12:48   ` Andrey Ryabinin
  0 siblings, 2 replies; 6+ messages in thread
From: Peter Zijlstra @ 2016-01-19 14:31 UTC (permalink / raw)
  To: Sasha Levin; +Cc: Ingo Molnar, Arnaldo Carvalho de Melo, LKML

On Sun, Jan 10, 2016 at 03:55:13PM -0500, Sasha Levin wrote:
> Hi all,
> 
> While fuzzing with trinity inside a KVM tools guest, running the latest -next
> kernel, I've hit the following warning:
> 
> [ 3494.030114] UBSAN: Undefined behaviour in kernel/events/ring_buffer.c:685:22
> [ 3494.030647] shift exponent -1 is negative

That's rb->page_order == -1, which should 'never' happen, curious!

Funny though that rb::page_order is the exact field _after_ rb::work, ho
humm.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: perf/ring-buffer: Undefined behaviour in kernel/events/ring_buffer.c:685:22
  2016-01-19 14:31 ` Peter Zijlstra
@ 2016-01-21 23:34   ` Sasha Levin
  2016-01-22 12:48   ` Andrey Ryabinin
  1 sibling, 0 replies; 6+ messages in thread
From: Sasha Levin @ 2016-01-21 23:34 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: Ingo Molnar, Arnaldo Carvalho de Melo, LKML

On 01/19/2016 09:31 AM, Peter Zijlstra wrote:
> On Sun, Jan 10, 2016 at 03:55:13PM -0500, Sasha Levin wrote:
>> Hi all,
>>
>> While fuzzing with trinity inside a KVM tools guest, running the latest -next
>> kernel, I've hit the following warning:
>>
>> [ 3494.030114] UBSAN: Undefined behaviour in kernel/events/ring_buffer.c:685:22
>> [ 3494.030647] shift exponent -1 is negative
> 
> That's rb->page_order == -1, which should 'never' happen, curious!
> 
> Funny though that rb::page_order is the exact field _after_ rb::work, ho
> humm.
> 

I've tested your theory using:

diff --git a/kernel/events/internal.h b/kernel/events/internal.h
index 2bbad9c..f627a40 100644
--- a/kernel/events/internal.h
+++ b/kernel/events/internal.h
@@ -14,6 +14,7 @@ struct ring_buffer {
        struct irq_work                 irq_work;
 #ifdef CONFIG_PERF_USE_VMALLOC
        struct work_struct              work;
+       unsigned long dummy;
        int                             page_order;     /* allocation order  */
 #endif
        int                             nr_pages;       /* nr of data pages  */
diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
index adfdc05..65346f8 100644
--- a/kernel/events/ring_buffer.c
+++ b/kernel/events/ring_buffer.c
@@ -682,6 +682,8 @@ void rb_free(struct ring_buffer *rb)
 #else
 static int data_page_nr(struct ring_buffer *rb)
 {
+       if (page_order(rb) < 0 || rb->dummy)
+               pr_emerg("*** %lx\n", rb->dummy);
        return rb->nr_pages << page_order(rb);
 }

But the output I'm seeing indicates that dummy isn't corrupted:

[  758.806091] *** 0
[  758.806821] ================================================================================
[  758.807961] UBSAN: Undefined behaviour in kernel/events/ring_buffer.c:687:22
[  758.808833] shift exponent -1 is negative
[...]


Thanks,
Sasha

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: perf/ring-buffer: Undefined behaviour in kernel/events/ring_buffer.c:685:22
  2016-01-19 14:31 ` Peter Zijlstra
  2016-01-21 23:34   ` Sasha Levin
@ 2016-01-22 12:48   ` Andrey Ryabinin
  2016-01-29 14:17     ` Peter Zijlstra
  1 sibling, 1 reply; 6+ messages in thread
From: Andrey Ryabinin @ 2016-01-22 12:48 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: Sasha Levin, Ingo Molnar, Arnaldo Carvalho de Melo, LKML

2016-01-19 17:31 GMT+03:00 Peter Zijlstra <peterz@infradead.org>:
> On Sun, Jan 10, 2016 at 03:55:13PM -0500, Sasha Levin wrote:
>> Hi all,
>>
>> While fuzzing with trinity inside a KVM tools guest, running the latest -next
>> kernel, I've hit the following warning:
>>
>> [ 3494.030114] UBSAN: Undefined behaviour in kernel/events/ring_buffer.c:685:22
>> [ 3494.030647] shift exponent -1 is negative
>
> That's rb->page_order == -1, which should 'never' happen, curious!
>

It happens if nr_pages = 0:
     rb->page_order = ilog2(nr_pages);

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: perf/ring-buffer: Undefined behaviour in kernel/events/ring_buffer.c:685:22
  2016-01-22 12:48   ` Andrey Ryabinin
@ 2016-01-29 14:17     ` Peter Zijlstra
  2016-03-21  9:51       ` [tip:perf/urgent] perf/core: Fix Undefined behaviour in rb_alloc() tip-bot for Peter Zijlstra
  0 siblings, 1 reply; 6+ messages in thread
From: Peter Zijlstra @ 2016-01-29 14:17 UTC (permalink / raw)
  To: Andrey Ryabinin; +Cc: Sasha Levin, Ingo Molnar, Arnaldo Carvalho de Melo, LKML

On Fri, Jan 22, 2016 at 03:48:55PM +0300, Andrey Ryabinin wrote:
> 2016-01-19 17:31 GMT+03:00 Peter Zijlstra <peterz@infradead.org>:
> > On Sun, Jan 10, 2016 at 03:55:13PM -0500, Sasha Levin wrote:
> >> Hi all,
> >>
> >> While fuzzing with trinity inside a KVM tools guest, running the latest -next
> >> kernel, I've hit the following warning:
> >>
> >> [ 3494.030114] UBSAN: Undefined behaviour in kernel/events/ring_buffer.c:685:22
> >> [ 3494.030647] shift exponent -1 is negative
> >
> > That's rb->page_order == -1, which should 'never' happen, curious!
> >
> 
> It happens if nr_pages = 0:
>      rb->page_order = ilog2(nr_pages);

Ah indeed.

Something like so then. When !nr_pages, both variables should be 0 and
are due to the kzalloc() of rb slightly above.

---
 kernel/events/ring_buffer.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
index adfdc0536117..345130705c49 100644
--- a/kernel/events/ring_buffer.c
+++ b/kernel/events/ring_buffer.c
@@ -746,8 +746,10 @@ struct ring_buffer *rb_alloc(int nr_pages, long watermark, int cpu, int flags)
 
 	rb->user_page = all_buf;
 	rb->data_pages[0] = all_buf + PAGE_SIZE;
-	rb->page_order = ilog2(nr_pages);
-	rb->nr_pages = !!nr_pages;
+	if (nr_pages) {
+		rb->nr_pages = 1;
+		rb->page_order = ilog2(nr_pages);
+	}
 
 	ring_buffer_init(rb, watermark, flags);
 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [tip:perf/urgent] perf/core: Fix Undefined behaviour in rb_alloc()
  2016-01-29 14:17     ` Peter Zijlstra
@ 2016-03-21  9:51       ` tip-bot for Peter Zijlstra
  0 siblings, 0 replies; 6+ messages in thread
From: tip-bot for Peter Zijlstra @ 2016-03-21  9:51 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: vincent.weaver, sasha.levin, acme, bp, acme, luto, namhyung,
	eranian, peterz, tglx, jolsa, brgerst, hpa, alexander.shishkin,
	dsahern, linux-kernel, mingo, dvlasenk, ryabinin.a.a, torvalds

Commit-ID:  8184059e93c200757f5c0805dae0f14e880eab5d
Gitweb:     http://git.kernel.org/tip/8184059e93c200757f5c0805dae0f14e880eab5d
Author:     Peter Zijlstra <peterz@infradead.org>
AuthorDate: Fri, 29 Jan 2016 15:17:51 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 21 Mar 2016 09:08:18 +0100

perf/core: Fix Undefined behaviour in rb_alloc()

Sasha reported:

 [ 3494.030114] UBSAN: Undefined behaviour in kernel/events/ring_buffer.c:685:22
 [ 3494.030647] shift exponent -1 is negative

Andrey spotted that this is because:

  It happens if nr_pages = 0:
     rb->page_order = ilog2(nr_pages);

Fix it by making both assignments conditional on nr_pages; since
otherwise they should both be 0 anyway, and will be because of the
kzalloc() used to allocate the structure.

Reported-by: Sasha Levin <sasha.levin@oracle.com>
Reported-by: Andrey Ryabinin <ryabinin.a.a@gmail.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vince Weaver <vincent.weaver@maine.edu>
Link: http://lkml.kernel.org/r/20160129141751.GA407@worktop
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/events/ring_buffer.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
index 1faad2c..c61f0cb 100644
--- a/kernel/events/ring_buffer.c
+++ b/kernel/events/ring_buffer.c
@@ -746,8 +746,10 @@ struct ring_buffer *rb_alloc(int nr_pages, long watermark, int cpu, int flags)
 
 	rb->user_page = all_buf;
 	rb->data_pages[0] = all_buf + PAGE_SIZE;
-	rb->page_order = ilog2(nr_pages);
-	rb->nr_pages = !!nr_pages;
+	if (nr_pages) {
+		rb->nr_pages = 1;
+		rb->page_order = ilog2(nr_pages);
+	}
 
 	ring_buffer_init(rb, watermark, flags);
 

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-03-21  9:55 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-10 20:55 perf/ring-buffer: Undefined behaviour in kernel/events/ring_buffer.c:685:22 Sasha Levin
2016-01-19 14:31 ` Peter Zijlstra
2016-01-21 23:34   ` Sasha Levin
2016-01-22 12:48   ` Andrey Ryabinin
2016-01-29 14:17     ` Peter Zijlstra
2016-03-21  9:51       ` [tip:perf/urgent] perf/core: Fix Undefined behaviour in rb_alloc() tip-bot for Peter Zijlstra

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.