New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 679490 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug-Security



Sign in to add a comment

Security: CVE-2016-9754

Project Member Reported by groeck@chromium.org, Jan 9 2017

Issue description

Advisory: CVE-2016-9754
  Details: https://web.nvd.nist.gov/view/vuln/detail?vulnId=CVE-2016-9754
  CVSS severity score: 7.8/7.2
  Description: 

The ring_buffer_resize function in kernel/trace/ring_buffer.c in the profiling subsystem in the Linux kernel before 4.6.1 mishandles certain integer calculations, which allows local users to gain privileges by writing to the /sys/kernel/debug/tracing/buffer_size_kb file.

 
Components: OS>Kernel
Labels: OS-Linux
Status: Started (was: Assigned)
Project Member

Comment 4 by bugdroid1@chromium.org, Jan 11 2017

Labels: merge-merged-chromeos-3.10
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/0b178201ea79c803450a6b576bd299b5bb2893db

commit 0b178201ea79c803450a6b576bd299b5bb2893db
Author: Steven Rostedt (Red Hat) <rostedt@goodmis.org>
Date: Fri May 13 13:34:12 2016

UPSTREAM: ring-buffer: Prevent overflow of size in ring_buffer_resize()

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.

BUG= chromium:679490 
TEST=Build and run basic tests

Change-Id: Id1952e43fd481f680999ff0ec409973f38136258
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>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
(cherry picked from commit 59643d1535eb)
Reviewed-on: https://chromium-review.googlesource.com/426358
Reviewed-by: Bernie Thompson <bhthompson@chromium.org>

[modify] https://crrev.com/0b178201ea79c803450a6b576bd299b5bb2893db/kernel/trace/ring_buffer.c

Project Member

Comment 5 by bugdroid1@chromium.org, Jan 11 2017

Labels: merge-merged-chromeos-3.18
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/3287ba5e6d2b751cb7e8db7175b2a93b54ffcd6f

commit 3287ba5e6d2b751cb7e8db7175b2a93b54ffcd6f
Author: Steven Rostedt (Red Hat) <rostedt@goodmis.org>
Date: Fri May 13 13:34:12 2016

UPSTREAM: ring-buffer: Prevent overflow of size in ring_buffer_resize()

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.

BUG= chromium:679490 
TEST=Build and run basic tests

Change-Id: Id1952e43fd481f680999ff0ec409973f38136258
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>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
(cherry picked from commit 59643d1535eb)
Reviewed-on: https://chromium-review.googlesource.com/426096
Reviewed-by: Bernie Thompson <bhthompson@chromium.org>

[modify] https://crrev.com/3287ba5e6d2b751cb7e8db7175b2a93b54ffcd6f/kernel/trace/ring_buffer.c

Project Member

Comment 6 by bugdroid1@chromium.org, Jan 11 2017

Labels: merge-merged-chromeos-3.14
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/3225b483e38e4fb8cf481e2ebfc22251f4fe7bee

commit 3225b483e38e4fb8cf481e2ebfc22251f4fe7bee
Author: Steven Rostedt (Red Hat) <rostedt@goodmis.org>
Date: Fri May 13 13:34:12 2016

UPSTREAM: ring-buffer: Prevent overflow of size in ring_buffer_resize()

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.

BUG= chromium:679490 
TEST=Build and run basic tests

Change-Id: Id1952e43fd481f680999ff0ec409973f38136258
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>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
(cherry picked from commit 59643d1535eb)
Reviewed-on: https://chromium-review.googlesource.com/426097
Reviewed-by: Bernie Thompson <bhthompson@chromium.org>

[modify] https://crrev.com/3225b483e38e4fb8cf481e2ebfc22251f4fe7bee/kernel/trace/ring_buffer.c

Comment 7 by groeck@chromium.org, Jan 11 2017

Labels: -Type-Bug Type-Bug-Security
Project Member

Comment 8 by sheriffbot@chromium.org, Jan 12 2017

Status: Fixed (was: Started)
Please mark security bugs as fixed as soon as the fix lands, and before requesting merges. This update is based on the merge- labels applied to this issue. Please reopen if this update was incorrect.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 9 by bugdroid1@chromium.org, Jan 13 2017

Labels: merge-merged-chromeos-3.8
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/f7e94588f838fd343eaeaafada8d9e3460dc2a8d

commit f7e94588f838fd343eaeaafada8d9e3460dc2a8d
Author: Steven Rostedt (Red Hat) <rostedt@goodmis.org>
Date: Fri May 13 13:34:12 2016

UPSTREAM: ring-buffer: Prevent overflow of size in ring_buffer_resize()

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.

BUG= chromium:679490 
TEST=Build and run basic tests

Change-Id: Id1952e43fd481f680999ff0ec409973f38136258
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>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
(cherry picked from commit 59643d1535eb)
Reviewed-on: https://chromium-review.googlesource.com/426359
Reviewed-by: Bernie Thompson <bhthompson@chromium.org>

[modify] https://crrev.com/f7e94588f838fd343eaeaafada8d9e3460dc2a8d/kernel/trace/ring_buffer.c

Labels: Merge-Request-56
Project Member

Comment 11 by sheriffbot@chromium.org, Jan 13 2017

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: -Merge-Request-56 Merge-Approved-56
Project Member

Comment 13 by bugdroid1@chromium.org, Jan 13 2017

Labels: merge-merged-release-R56-9000.B-chromeos-3.18
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/6695c99d2861da6cc32dde357098b9ba5055da67

commit 6695c99d2861da6cc32dde357098b9ba5055da67
Author: Steven Rostedt (Red Hat) <rostedt@goodmis.org>
Date: Fri May 13 13:34:12 2016

UPSTREAM: ring-buffer: Prevent overflow of size in ring_buffer_resize()

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.

BUG= chromium:679490 
TEST=Build and run basic tests

Change-Id: Id1952e43fd481f680999ff0ec409973f38136258
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>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
(cherry picked from commit 59643d1535eb)
Reviewed-on: https://chromium-review.googlesource.com/426096
Reviewed-by: Bernie Thompson <bhthompson@chromium.org>
(cherry picked from commit 3287ba5e6d2b751cb7e8db7175b2a93b54ffcd6f)
Reviewed-on: https://chromium-review.googlesource.com/428150

[modify] https://crrev.com/6695c99d2861da6cc32dde357098b9ba5055da67/kernel/trace/ring_buffer.c

Project Member

Comment 14 by bugdroid1@chromium.org, Jan 13 2017

Labels: merge-merged-release-R56-9000.B-chromeos-3.18
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/6695c99d2861da6cc32dde357098b9ba5055da67

commit 6695c99d2861da6cc32dde357098b9ba5055da67
Author: Steven Rostedt (Red Hat) <rostedt@goodmis.org>
Date: Fri May 13 13:34:12 2016

UPSTREAM: ring-buffer: Prevent overflow of size in ring_buffer_resize()

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.

BUG= chromium:679490 
TEST=Build and run basic tests

Change-Id: Id1952e43fd481f680999ff0ec409973f38136258
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>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
(cherry picked from commit 59643d1535eb)
Reviewed-on: https://chromium-review.googlesource.com/426096
Reviewed-by: Bernie Thompson <bhthompson@chromium.org>
(cherry picked from commit 3287ba5e6d2b751cb7e8db7175b2a93b54ffcd6f)
Reviewed-on: https://chromium-review.googlesource.com/428150

[modify] https://crrev.com/6695c99d2861da6cc32dde357098b9ba5055da67/kernel/trace/ring_buffer.c

Project Member

Comment 15 by bugdroid1@chromium.org, Jan 13 2017

Labels: merge-merged-release-R56-9000.B-chromeos-3.14
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/0ccea348cd7db6bab5ed5c5cfbf1c8d82e4b5d2f

commit 0ccea348cd7db6bab5ed5c5cfbf1c8d82e4b5d2f
Author: Steven Rostedt (Red Hat) <rostedt@goodmis.org>
Date: Fri May 13 13:34:12 2016

UPSTREAM: ring-buffer: Prevent overflow of size in ring_buffer_resize()

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.

BUG= chromium:679490 
TEST=Build and run basic tests

Change-Id: Id1952e43fd481f680999ff0ec409973f38136258
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>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
(cherry picked from commit 59643d1535eb)
Reviewed-on: https://chromium-review.googlesource.com/426097
Reviewed-by: Bernie Thompson <bhthompson@chromium.org>
(cherry picked from commit 3225b483e38e4fb8cf481e2ebfc22251f4fe7bee)
Reviewed-on: https://chromium-review.googlesource.com/428151

[modify] https://crrev.com/0ccea348cd7db6bab5ed5c5cfbf1c8d82e4b5d2f/kernel/trace/ring_buffer.c

Project Member

Comment 16 by bugdroid1@chromium.org, Jan 13 2017

Labels: merge-merged-release-R56-9000.B-chromeos-3.10
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/1eebe41b56b692876f2615465a67ebc22e8a2e60

commit 1eebe41b56b692876f2615465a67ebc22e8a2e60
Author: Steven Rostedt (Red Hat) <rostedt@goodmis.org>
Date: Fri May 13 13:34:12 2016

UPSTREAM: ring-buffer: Prevent overflow of size in ring_buffer_resize()

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.

BUG= chromium:679490 
TEST=Build and run basic tests

Change-Id: Id1952e43fd481f680999ff0ec409973f38136258
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>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
(cherry picked from commit 59643d1535eb)
Reviewed-on: https://chromium-review.googlesource.com/426358
Reviewed-by: Bernie Thompson <bhthompson@chromium.org>
(cherry picked from commit 0b178201ea79c803450a6b576bd299b5bb2893db)
Reviewed-on: https://chromium-review.googlesource.com/428152

[modify] https://crrev.com/1eebe41b56b692876f2615465a67ebc22e8a2e60/kernel/trace/ring_buffer.c

Project Member

Comment 17 by bugdroid1@chromium.org, Jan 13 2017

Labels: merge-merged-release-R56-9000.B-chromeos-3.10
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/1eebe41b56b692876f2615465a67ebc22e8a2e60

commit 1eebe41b56b692876f2615465a67ebc22e8a2e60
Author: Steven Rostedt (Red Hat) <rostedt@goodmis.org>
Date: Fri May 13 13:34:12 2016

UPSTREAM: ring-buffer: Prevent overflow of size in ring_buffer_resize()

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.

BUG= chromium:679490 
TEST=Build and run basic tests

Change-Id: Id1952e43fd481f680999ff0ec409973f38136258
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>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
(cherry picked from commit 59643d1535eb)
Reviewed-on: https://chromium-review.googlesource.com/426358
Reviewed-by: Bernie Thompson <bhthompson@chromium.org>
(cherry picked from commit 0b178201ea79c803450a6b576bd299b5bb2893db)
Reviewed-on: https://chromium-review.googlesource.com/428152

[modify] https://crrev.com/1eebe41b56b692876f2615465a67ebc22e8a2e60/kernel/trace/ring_buffer.c

Project Member

Comment 18 by bugdroid1@chromium.org, Jan 13 2017

Labels: merge-merged-release-R56-9000.B-chromeos-3.8
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/96be95bbfc84969b84397c722afc20ad45e829d2

commit 96be95bbfc84969b84397c722afc20ad45e829d2
Author: Steven Rostedt (Red Hat) <rostedt@goodmis.org>
Date: Fri May 13 13:34:12 2016

UPSTREAM: ring-buffer: Prevent overflow of size in ring_buffer_resize()

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.

BUG= chromium:679490 
TEST=Build and run basic tests

Change-Id: Id1952e43fd481f680999ff0ec409973f38136258
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>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
(cherry picked from commit 59643d1535eb)
Reviewed-on: https://chromium-review.googlesource.com/426359
Reviewed-by: Bernie Thompson <bhthompson@chromium.org>
(cherry picked from commit f7e94588f838fd343eaeaafada8d9e3460dc2a8d)
Reviewed-on: https://chromium-review.googlesource.com/428153

[modify] https://crrev.com/96be95bbfc84969b84397c722afc20ad45e829d2/kernel/trace/ring_buffer.c

Project Member

Comment 19 by bugdroid1@chromium.org, Jan 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/96be95bbfc84969b84397c722afc20ad45e829d2

commit 96be95bbfc84969b84397c722afc20ad45e829d2
Author: Steven Rostedt (Red Hat) <rostedt@goodmis.org>
Date: Fri May 13 13:34:12 2016

UPSTREAM: ring-buffer: Prevent overflow of size in ring_buffer_resize()

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.

BUG= chromium:679490 
TEST=Build and run basic tests

Change-Id: Id1952e43fd481f680999ff0ec409973f38136258
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>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
(cherry picked from commit 59643d1535eb)
Reviewed-on: https://chromium-review.googlesource.com/426359
Reviewed-by: Bernie Thompson <bhthompson@chromium.org>
(cherry picked from commit f7e94588f838fd343eaeaafada8d9e3460dc2a8d)
Reviewed-on: https://chromium-review.googlesource.com/428153

[modify] https://crrev.com/96be95bbfc84969b84397c722afc20ad45e829d2/kernel/trace/ring_buffer.c

Project Member

Comment 20 by sheriffbot@chromium.org, Jan 17 2017

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Approved-56
Project Member

Comment 22 by sheriffbot@chromium.org, Apr 21 2017

Labels: -Restrict-View-SecurityNotify allpublic
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Sign in to add a comment