heap overflows in nvidia driver

Bug #979373 reported by Kees Cook on 2012-04-11
278
This bug affects 2 people
Affects Status Importance Assigned to Milestone
nvidia-graphics-drivers (Ubuntu)
High
Unassigned

Bug Description

Hello,

While I haven't had time to prove these out, they seem to be problems.

1) Race, available only to uid 0 (but uid 0 should not mean ring-0 access): /proc/driver/nvidia/registry is vulnerable to a write race that can result in a limited heap overflow.
nv_procfs_write_registry does not perform locking on nvfp->off, so two writers (of the same fd) could race to confuse the bytes_left and proc_buffer locations, leading to heap overflow:

writer 1: bytes_left = (NV_PROC_WRITE_BUFFER_SIZE - nvfp->off - 1);
writer 1: ...
writer 1: copy_from_user(proc_buffer, buffer, count))
writer 2: bytes_left = (NV_PROC_WRITE_BUFFER_SIZE - nvfp->off - 1);
writer 1: nvfp->off += count;
writer 2: ...
writer 2: proc_buffer = &((char *)nvfp->data)[nvfp->off];
writer 2: copy_from_user(proc_buffer, buffer, count)

writer 2's count was checked against nvfp->off before it was moved, and writer 2's proc_buffer is now offset by nvfp->off, allowing a write past the end of the heap buffer, by at most NV_PROC_WRITE_BUFFER_SIZE (64k) - 2 bytes.

2) Heap overflow in control device ioctl: minimum size of the ioctl buffer is not checked for NV_ESC_CARD_INFO, which will write 50 bytes per device to the allocated kernel buffer (which was sized to the input buffer), before attempting to then write it back to the user buffer. With a minimum 1 byte buffer, this is a 49 byte overflow, since the rm_api->magic check doesn't actually abort the ioctl.

I expect there are additional heap overflows in the rm_ioctl function since it is not passed arg_size as a parameter, but I didn't have time to examine the binary module. Seems like enforcing a minimum allocation size when calling rm_ioctl would be the simplest fix.

3) Kernel heap contents leak race in ioctl handler: the ioctl will copy the contents of kernel heap back to the user buffer even on failure. By racing the ioctl with a change in VMA protections, it should be possible to extract uncleared kernel heap memory:

thread 1: set VMA for arg_ptr to PROT_NONE
thread 1: NV_KMALLOC(arg_copy, arg_size);
thread 1: copy_from_user(arg_copy, arg_ptr, arg_size) <- fails and jumps to "done"
thread 2: set VMA for arg_ptr to PROT_WRITE
thread 1: if (arg_copy != NULL) ... copy_to_user(arg_ptr, arg_copy, arg_size)

CVE References

Bryce Harrington (bryce) wrote :

@Alberto, please escalate with NVIDIA as soon as possible.

Jamie Strandboge (jdstrand) wrote :

Is there any word on this?

Daniel Dadap (ddadap) wrote :

Thanks for reporting these issues. We are investigating each of the three reported issues separately.

Bryce Harrington (bryce) on 2012-05-07
Changed in nvidia-graphics-drivers (Ubuntu):
importance: Undecided → High
status: New → Triaged
Daniel Dadap (ddadap) wrote :

Hi everybody,

All three of these potential races/overflows should be resolved in the new 295.53 driver. Please have the original reporter verify that the resolution is satisfactory for all three issues.

Kees Cook (kees) wrote :

Thanks for working on this! Is there a patch I can review?

Daniel Dadap (ddadap) wrote :

You should be able to just diff the kernel module interface layer sources between 295.49 and 295.53. I'll pull the actual individual changes to make review easier, and to filter away any unrelated changes.

Daniel Dadap (ddadap) wrote :

Sorry, it turned out that one of the changes required changes to non open-source portions of the driver code. They are pretty basic changes, but I'm not authorized to release the complete patch. I will post sanitized patches, which only include the open-source changes, shortly.

Daniel Dadap (ddadap) wrote :

The patch "proclock.diff" addresses problem (1) identified here.

The patch "ioctl.diff" addresses problems (2) and (3). Unfortunately, this patch is heavily redacted, due to the bulk of the changes having been made in non-open source parts of the driver. Note that changes to the Solaris interface layer are also redacted, due to the licensing for the Solaris driver: the Solaris interface layer changes are functionally equivalent to the Linux and FreeBSD changes.

Daniel Dadap (ddadap) wrote :

Second patch, since launchpad only allows one attachment per comment

Thanks very much for getting these fixed and providing the patches for
review. :)

--
Kees Cook

Marc Deslauriers (mdeslaur) wrote :

(1) is CVE-2012-0951:
/proc/driver/nvidia/registry is vulnerable to a write race that can result in a limited heap overflow.

(2) is CVE-2012-0952:
Heap overflow in control device ioctl

(3) is CVE-2012-0953:
Kernel heap contents leak race in ioctl handler

Marc Deslauriers (mdeslaur) wrote :

@Daniel:

So, have these been fixed in 295.59 now? Should we update all Ubuntu releases to that version?

What about the legacy GPU trees, have they been updated also?

Kees Cook (kees) on 2012-07-13
visibility: private → public

The attachment "Patch for problem (1)" of this bug report has been identified as being a patch. The ubuntu-reviewers team has been subscribed to the bug report so that they can review the patch. In the event that this is in fact not a patch you can resolve this situation by removing the tag 'patch' from the bug report and editing the attachment so that it is not flagged as a patch. Additionally, if you are member of the ubuntu-reviewers team please also unsubscribe the team from this bug report.

[This is an automated message performed by a Launchpad user owned by Brian Murray. Please contact him regarding any issues with the action taken in this bug report.]

tags: added: patch
Marc Deslauriers (mdeslaur) wrote :

All supported releases have been bumped to the 304.x drivers, so this is fix now. Closing.

Changed in nvidia-graphics-drivers (Ubuntu):
status: Triaged → Fix Released
To post a comment you must log in.
This report contains Public Security information  Edit
Everyone can see this security related information.

Other bug subscribers