ACK: [Trusty SRU][PATCH 1/1] ring-buffer: Prevent overflow of size in ring_buffer_resize()
Stefan Bader
stefan.bader at canonical.com
Tue Aug 29 08:11:31 UTC 2017
On 25.08.2017 18:57, Kleber Sacilotto de Souza wrote:
> From: "Steven Rostedt (Red Hat)" <rostedt at 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 at vger.kernel.org # 3.5+
> Fixes: 83f40318dab00 ("ring-buffer: Make removal of ring buffer pages atomic")
> Signed-off-by: Steven Rostedt <rostedt at goodmis.org>
> (cherry picked from commit 59643d1535eb220668692a5359de22545af579f6)
> Signed-off-by: Kleber Sacilotto de Souza <kleber.souza at canonical.com>
Acked-by: Stefan Bader <stefan.bader at 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
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20170829/2285036f/attachment.sig>
More information about the kernel-team
mailing list