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

Issue 609382 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Jun 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug-Security



Sign in to add a comment

Security: Use after free of task_struct in Mali Midgard driver.

Project Member Reported by rickyz@chromium.org, May 5 2016

Issue description

In mali_kbase_mem_linux.c, kbase_mem_from_user_buffer does:

reg->gpu_alloc->imported.user_buf.owner = current;

This is later used in kbase_jd_user_buf_map in mali_kbase_jd.c:

owner = alloc->imported.user_buf.owner;
...
down_read(&owner->mm->mmap_sem);
pinned_pages = get_user_pages(owner, owner->mm,
                address,
                alloc->imported.user_buf.nr_pages,
                reg->flags & KBASE_REG_GPU_WR,
                0, pages, NULL);
up_read(&owner->mm->mmap_sem);

If a program imports a user buffer on a thread, then kills that thread, the freed task_struct for that thread will be used when the program submits a job depending on the buffer.

I've attached a program which crashes on the down_read on my test device. I not written an exploit, but I suspect that this can be used to escalate privileges with some work. The rough idea would be to get the task_struct of a more privileged task (like a setuid binary) at owner. The code would then expose pages from that task to the GPU, which could then read/write them.

I mostly copied over the CC list from  issue 601801 , so apologies if you're getting this and it's not relevant.

 
test.c
5.4 KB View Download
ARM Ref: MIDCET-1300

Comment 3 by orjan.e...@arm.com, May 10 2016

Thank you for reporting this!

Fixes have been pushed here:
chromeos-3.14: https://chromium-review.googlesource.com/343862
chromeos-3.8: https://chromium-review.googlesource.com/343880

These have been tested with the attached test.c (which doesn't crash the kernel after the fix), and by booting to UI and running aquarium.
Project Member

Comment 4 by bugdroid1@chromium.org, May 14 2016

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

commit 166ced60a408bedfdf573ad30c34c89a5e302e99
Author: Ørjan Eide <orjan.eide@arm.com>
Date: Mon May 09 12:47:47 2016

MALI: Fix possible use after free of task_struct

Move to using the mm from the task, with reference counting, instead of
storing the task_struct pointer.

BUG= chromium:609382 
TEST=run aquarium on daisy
TEST=run test.c from ticket

Change-Id: I4f8d1761a2286c5e8bce829ee3ff5aa2826fdf65
Reviewed-on: https://chromium-review.googlesource.com/343880
Commit-Ready: Stéphane Marchesin <marcheu@chromium.org>
Tested-by: Stéphane Marchesin <marcheu@chromium.org>
Reviewed-by: Stéphane Marchesin <marcheu@chromium.org>
Reviewed-by: Ricky Zhou <rickyz@chromium.org>

[modify] https://crrev.com/166ced60a408bedfdf573ad30c34c89a5e302e99/drivers/gpu/arm/midgard/mali_kbase_jd.c
[modify] https://crrev.com/166ced60a408bedfdf573ad30c34c89a5e302e99/drivers/gpu/arm/midgard/mali_kbase_mem.h
[modify] https://crrev.com/166ced60a408bedfdf573ad30c34c89a5e302e99/drivers/gpu/arm/midgard/mali_kbase_mem.c
[modify] https://crrev.com/166ced60a408bedfdf573ad30c34c89a5e302e99/drivers/gpu/arm/midgard/mali_kbase_mem_linux.c

Project Member

Comment 5 by bugdroid1@chromium.org, May 14 2016

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

commit 5f3b6b6d12fe6a358ffb855695a80d1116b8fc95
Author: Ørjan Eide <orjan.eide@arm.com>
Date: Mon May 09 12:47:47 2016

MALI: Fix possible use after free of task_struct

Move to using the mm from the task, with reference counting, instead of
storing the task_struct pointer.

BUG= chromium:609382 
TEST=run aquarium on jaq
TEST=run test.c from ticket

Change-Id: I4f8d1761a2286c5e8bce829ee3ff5aa2826fdf65
Reviewed-on: https://chromium-review.googlesource.com/343862
Commit-Ready: Stéphane Marchesin <marcheu@chromium.org>
Tested-by: Stéphane Marchesin <marcheu@chromium.org>
Reviewed-by: Stéphane Marchesin <marcheu@chromium.org>

[modify] https://crrev.com/5f3b6b6d12fe6a358ffb855695a80d1116b8fc95/drivers/gpu/arm/midgard/mali_kbase_jd.c
[modify] https://crrev.com/5f3b6b6d12fe6a358ffb855695a80d1116b8fc95/drivers/gpu/arm/midgard/mali_kbase_mem.h
[modify] https://crrev.com/5f3b6b6d12fe6a358ffb855695a80d1116b8fc95/drivers/gpu/arm/midgard/mali_kbase_mem.c
[modify] https://crrev.com/5f3b6b6d12fe6a358ffb855695a80d1116b8fc95/drivers/gpu/arm/midgard/mali_kbase_mem_linux.c

Project Member

Comment 6 by bugdroid1@chromium.org, May 17 2016

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

commit 07b393ee02244b715831fe589a9bfc3b3eee336b
Author: Ørjan Eide <orjan.eide@arm.com>
Date: Mon May 09 12:47:47 2016

MALI: Fix possible use after free of task_struct

Move to using the mm from the task, with reference counting, instead of
storing the task_struct pointer.

BUG= chromium:609382 
TEST=run aquarium on jaq
TEST=run test.c from ticket

Change-Id: I4f8d1761a2286c5e8bce829ee3ff5aa2826fdf65
Reviewed-on: https://chromium-review.googlesource.com/343862
Commit-Ready: Stphane Marchesin <marcheu@chromium.org>
Tested-by: Stphane Marchesin <marcheu@chromium.org>
Reviewed-by: Stphane Marchesin <marcheu@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/344758
Commit-Ready: Stéphane Marchesin <marcheu@chromium.org>
Tested-by: Stéphane Marchesin <marcheu@chromium.org>
Reviewed-by: Stéphane Marchesin <marcheu@chromium.org>

[modify] https://crrev.com/07b393ee02244b715831fe589a9bfc3b3eee336b/drivers/gpu/arm/midgard/mali_kbase_jd.c
[modify] https://crrev.com/07b393ee02244b715831fe589a9bfc3b3eee336b/drivers/gpu/arm/midgard/mali_kbase_mem.h
[modify] https://crrev.com/07b393ee02244b715831fe589a9bfc3b3eee336b/drivers/gpu/arm/midgard/mali_kbase_mem.c
[modify] https://crrev.com/07b393ee02244b715831fe589a9bfc3b3eee336b/drivers/gpu/arm/midgard/mali_kbase_mem_linux.c

Project Member

Comment 7 by sheriffbot@chromium.org, May 26 2016

Labels: -M-50 M-51
Anything we need from the ExternalDependency or can we move this to Fixed?

Comment 9 by rickyz@chromium.org, Jun 28 2016

Status: Fixed (was: ExternalDependency)
This should be fixed, thanks.
Project Member

Comment 10 by sheriffbot@chromium.org, Jun 29 2016

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: VerifyIn-54
Labels: -Restrict-View-SecurityNotify
Labels: Bulk-Verified
Status: Verified (was: Fixed)
Project Member

Comment 14 by sheriffbot@chromium.org, Oct 5 2016

Labels: 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