* 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 related [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 related [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 related [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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox