Message ID | 20170825165756.25240-2-kleber.souza@canonical.com |
---|---|
State | New |
Headers | show |
On 25.08.2017 18:57, Kleber Sacilotto de Souza wrote: > From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org> > > CVE-2016-9754 > > If the size passed to ring_buffer_resize() is greater than MAX_LONG - BUF_PAGE_SIZE > then the DIV_ROUND_UP() will return zero. > > Here's the details: > > # echo 18014398509481980 > /sys/kernel/debug/tracing/buffer_size_kb > > tracing_entries_write() processes this and converts kb to bytes. > > 18014398509481980 << 10 = 18446744073709547520 > > and this is passed to ring_buffer_resize() as unsigned long size. > > size = DIV_ROUND_UP(size, BUF_PAGE_SIZE); > > Where DIV_ROUND_UP(a, b) is (a + b - 1)/b > > BUF_PAGE_SIZE is 4080 and here > > 18446744073709547520 + 4080 - 1 = 18446744073709551599 > > where 18446744073709551599 is still smaller than 2^64 > > 2^64 - 18446744073709551599 = 17 > > But now 18446744073709551599 / 4080 = 4521260802379792 > > and size = size * 4080 = 18446744073709551360 > > This is checked to make sure its still greater than 2 * 4080, > which it is. > > Then we convert to the number of buffer pages needed. > > nr_page = DIV_ROUND_UP(size, BUF_PAGE_SIZE) > > but this time size is 18446744073709551360 and > > 2^64 - (18446744073709551360 + 4080 - 1) = -3823 > > Thus it overflows and the resulting number is less than 4080, which makes > > 3823 / 4080 = 0 > > an nr_pages is set to this. As we already checked against the minimum that > nr_pages may be, this causes the logic to fail as well, and we crash the > kernel. > > There's no reason to have the two DIV_ROUND_UP() (that's just result of > historical code changes), clean up the code and fix this bug. > > Cc: stable@vger.kernel.org # 3.5+ > Fixes: 83f40318dab00 ("ring-buffer: Make removal of ring buffer pages atomic") > Signed-off-by: Steven Rostedt <rostedt@goodmis.org> > (cherry picked from commit 59643d1535eb220668692a5359de22545af579f6) > Signed-off-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com> Acked-by: Stefan Bader <stefan.bader@canonical.com> > --- > kernel/trace/ring_buffer.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c > index c660e9feeb61..a936c9cb0687 100644 > --- a/kernel/trace/ring_buffer.c > +++ b/kernel/trace/ring_buffer.c > @@ -1641,14 +1641,13 @@ int ring_buffer_resize(struct ring_buffer *buffer, unsigned long size, > !cpumask_test_cpu(cpu_id, buffer->cpumask)) > return size; > > - size = DIV_ROUND_UP(size, BUF_PAGE_SIZE); > - size *= BUF_PAGE_SIZE; > + nr_pages = DIV_ROUND_UP(size, BUF_PAGE_SIZE); > > /* we need a minimum of two pages */ > - if (size < BUF_PAGE_SIZE * 2) > - size = BUF_PAGE_SIZE * 2; > + if (nr_pages < 2) > + nr_pages = 2; > > - nr_pages = DIV_ROUND_UP(size, BUF_PAGE_SIZE); > + size = nr_pages * BUF_PAGE_SIZE; > > /* > * Don't succeed if resizing is disabled, as a reader might be >
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c index c660e9feeb61..a936c9cb0687 100644 --- a/kernel/trace/ring_buffer.c +++ b/kernel/trace/ring_buffer.c @@ -1641,14 +1641,13 @@ int ring_buffer_resize(struct ring_buffer *buffer, unsigned long size, !cpumask_test_cpu(cpu_id, buffer->cpumask)) return size; - size = DIV_ROUND_UP(size, BUF_PAGE_SIZE); - size *= BUF_PAGE_SIZE; + nr_pages = DIV_ROUND_UP(size, BUF_PAGE_SIZE); /* we need a minimum of two pages */ - if (size < BUF_PAGE_SIZE * 2) - size = BUF_PAGE_SIZE * 2; + if (nr_pages < 2) + nr_pages = 2; - nr_pages = DIV_ROUND_UP(size, BUF_PAGE_SIZE); + size = nr_pages * BUF_PAGE_SIZE; /* * Don't succeed if resizing is disabled, as a reader might be