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