diff mbox

[RFC,v2,0/4] mm: Hardened usercopy

Message ID 16741.1466120212@turing-police.cc.vt.edu (mailing list archive)
State New, archived
Headers show

Commit Message

Valdis Klētnieks June 16, 2016, 11:36 p.m. UTC 
On Wed, 15 Jun 2016 18:38:31 -0700, Kees Cook said:
> On Wed, Jun 15, 2016 at 6:30 PM,  <Valdis.Kletnieks@vt.edu> wrote:
> > So I guess you can stick a:
> >
> > Tested-By: Valdis Kletnieks <valdis.kletnieks@vt.edu>
> >
> > on that patch set. :)
>
> Awesome, thanks! It's good to know the system operated normally up
> until that point. I'm glad to have lots of people testing.

Following up - I did a BFI patch against the NVidia driver that basically
tagged all its memory allocations as USERCOPY, and the resulting kernel
has gotten up to multiuser and XOrg.  Been up for a half hour doing my usual
stuff on the laptop, and no usercopy whines.

Workload: email, pidgin IM, Google Chrome with some 30 tabs, some streaming
audio. Plenty of room for corner cases still lurking, but all the basic
stuff is working.  I may whomp on it with trinity for a while, see if
anything falls out...

Today's surprise: VirtualBox 5.0.22 was released - and it was able to boot
a Windows 7 image to the desktop without complaint.  Something still wonky
there, as it gets unstable at some point, but given the lack of dmesg entries,
I suspect it's a linux-next regression rather than a usercopy issue.  Will
debug more later tonight.

NVidia patch attached as guidance to what's needed for anybody else who's facing
patching an out-of-tree module.

Comments

Valdis Klētnieks June 17, 2016, 1:38 a.m. UTC | #1 
On Thu, 16 Jun 2016 19:36:52 -0400, Valdis.Kletnieks@vt.edu said:

> stuff is working.  I may whomp on it with trinity for a while, see if
> anything falls out...

Woo hoo! Bagged one! :)  (Haven't figured out yet if actual bug, or missing
annotation)

[ 4033.178386] NET: Registered protocol family 21
[ 4033.226806] NET: Registered protocol family 38
[ 4033.256276] Guest personality initialized and is inactive
[ 4033.256797] VMCI host device registered (name=vmci, major=10, minor=53)
[ 4033.256801] Initialized host personality
[ 4033.266376] NET: Registered protocol family 40
[ 4033.365982] NET: Registered protocol family 24
[ 4033.413031] irda_setsockopt: not allowed to set MAXSDUSIZE for this socket type!
[ 4033.531569] sock: process `trinity-main' is using obsolete setsockopt SO_BSDCOMPAT
[ 4033.834839] irda_setsockopt: not allowed to set MAXSDUSIZE for this socket type!
[ 4034.444515] irda_setsockopt: not allowed to set MAXSDUSIZE for this socket type!
[ 4034.569913] sctp: [Deprecated]: trinity-main (pid 19154) Use of int in max_burst socket option deprecated.
[ 4034.569913] Use struct sctp_assoc_value instead
[ 4034.728723] usercopy: kernel memory overwrite attempt detected to ffff8801ecef4700 (SCTP) (4 bytes)
[ 4034.728730] CPU: 3 PID: 19154 Comm: trinity-main Tainted: G           OE   4.7.0-rc3-next-20160614-dirty #302
[ 4034.728732] Hardware name: Dell Inc. Latitude E6530/07Y85M, BIOS A17 08/19/2015
[ 4034.728734]  0000000000000000 0000000063913a95 ffff8801f8b33da8 ffffffffb269f61a
[ 4034.728740]  ffff8801ecef4700 0000000063913a95 0000000000000004 0000000000000000
[ 4034.728744]  ffff8801f8b33df8 ffffffffb2367b30 0000000000000004 ffffea0006bd4580
[ 4034.728748] Call Trace:
[ 4034.728754]  [<ffffffffb269f61a>] dump_stack+0x7b/0xd1
[ 4034.728758]  [<ffffffffb2367b30>] __check_object_size+0x70/0x3d4
[ 4034.728761]  [<ffffffffb2eae5e4>] sctp_setsockopt.part.9+0x684/0x1e70
[ 4034.728764]  [<ffffffffb236f002>] ? __vfs_write+0x22/0x2e0
[ 4034.728767]  [<ffffffffb2eafe3e>] sctp_setsockopt+0x6e/0xe0
[ 4034.728770]  [<ffffffffb2bd1d0a>] sock_common_setsockopt+0x3a/0xc0
[ 4034.728772]  [<ffffffffb2bcfb99>] SyS_setsockopt+0x89/0x120
[ 4034.728775]  [<ffffffffb30896e5>] entry_SYSCALL_64_fastpath+0x18/0xa8
[ 4034.728779]  [<ffffffffb2143e3f>] ? trace_hardirqs_off_caller+0x1f/0xf0

Do we have a good place to collect these, or should I just post them here
as I find stuff?
Kees Cook June 18, 2016, 7:30 p.m. UTC | #2 
On Thu, Jun 16, 2016 at 6:38 PM,  <Valdis.Kletnieks@vt.edu> wrote:
> On Thu, 16 Jun 2016 19:36:52 -0400, Valdis.Kletnieks@vt.edu said:
>
>> stuff is working.  I may whomp on it with trinity for a while, see if
>> anything falls out...
>
> Woo hoo! Bagged one! :)  (Haven't figured out yet if actual bug, or missing
> annotation)
>
> [ 4033.178386] NET: Registered protocol family 21
> [ 4033.226806] NET: Registered protocol family 38
> [ 4033.256276] Guest personality initialized and is inactive
> [ 4033.256797] VMCI host device registered (name=vmci, major=10, minor=53)
> [ 4033.256801] Initialized host personality
> [ 4033.266376] NET: Registered protocol family 40
> [ 4033.365982] NET: Registered protocol family 24
> [ 4033.413031] irda_setsockopt: not allowed to set MAXSDUSIZE for this socket type!
> [ 4033.531569] sock: process `trinity-main' is using obsolete setsockopt SO_BSDCOMPAT
> [ 4033.834839] irda_setsockopt: not allowed to set MAXSDUSIZE for this socket type!
> [ 4034.444515] irda_setsockopt: not allowed to set MAXSDUSIZE for this socket type!
> [ 4034.569913] sctp: [Deprecated]: trinity-main (pid 19154) Use of int in max_burst socket option deprecated.
> [ 4034.569913] Use struct sctp_assoc_value instead
> [ 4034.728723] usercopy: kernel memory overwrite attempt detected to ffff8801ecef4700 (SCTP) (4 bytes)
> [ 4034.728730] CPU: 3 PID: 19154 Comm: trinity-main Tainted: G           OE   4.7.0-rc3-next-20160614-dirty #302
> [ 4034.728732] Hardware name: Dell Inc. Latitude E6530/07Y85M, BIOS A17 08/19/2015
> [ 4034.728734]  0000000000000000 0000000063913a95 ffff8801f8b33da8 ffffffffb269f61a
> [ 4034.728740]  ffff8801ecef4700 0000000063913a95 0000000000000004 0000000000000000
> [ 4034.728744]  ffff8801f8b33df8 ffffffffb2367b30 0000000000000004 ffffea0006bd4580
> [ 4034.728748] Call Trace:
> [ 4034.728754]  [<ffffffffb269f61a>] dump_stack+0x7b/0xd1
> [ 4034.728758]  [<ffffffffb2367b30>] __check_object_size+0x70/0x3d4
> [ 4034.728761]  [<ffffffffb2eae5e4>] sctp_setsockopt.part.9+0x684/0x1e70
> [ 4034.728764]  [<ffffffffb236f002>] ? __vfs_write+0x22/0x2e0
> [ 4034.728767]  [<ffffffffb2eafe3e>] sctp_setsockopt+0x6e/0xe0
> [ 4034.728770]  [<ffffffffb2bd1d0a>] sock_common_setsockopt+0x3a/0xc0
> [ 4034.728772]  [<ffffffffb2bcfb99>] SyS_setsockopt+0x89/0x120
> [ 4034.728775]  [<ffffffffb30896e5>] entry_SYSCALL_64_fastpath+0x18/0xa8
> [ 4034.728779]  [<ffffffffb2143e3f>] ? trace_hardirqs_off_caller+0x1f/0xf0

Cool, interesting. I don't see anything obvious in grsecurity's
patches that covers this, so either I'm missing something else, or
this bug exists there too. (Though not a lot of people use SCTP,
though.)

> Do we have a good place to collect these, or should I just post them here
> as I find stuff?

For now, let's just collect them on the list, and any patches that
might solve them. I'm hoping to add the copy_*_user_n() API to help
with these.

-Kees
diff mbox

Patch

--- nvidia-uvm/uvm_linux.h.dist	2016-06-16 04:54:42.573247324 -0400
+++ nvidia-uvm/uvm_linux.h	2016-06-16 17:23:29.863108182 -0400
@@ -185,7 +185,11 @@ 
 #define __GFP_NORETRY 0
 #endif

-#define NV_UVM_GFP_FLAGS (GFP_KERNEL | __GFP_NORETRY)
+#if !defined(__GFP_USERCOPY)
+#define __GFP_USERCOPY 0
+#endif
+
+#define NV_UVM_GFP_FLAGS (GFP_KERNEL | __GFP_NORETRY | __GFP_USERCOPY)

 #if defined(NV_VM_INSERT_PAGE_PRESENT)
 #define NV_VM_INSERT_PAGE(vma, addr, page) \
--- nvidia/nv-vm.c.dist	2016-06-09 20:37:13.000000000 -0400
+++ nvidia/nv-vm.c	2016-06-16 17:32:51.357212907 -0400
@@ -265,6 +265,9 @@ 
     if (at->flags & NV_ALLOC_TYPE_ZEROED)
         gfp_mask |= __GFP_ZERO;
 #endif
+#if defined(__GPF_USERCOPY)
+    gfp_mask |= __GFP_USERCOPY;
+#endif

     return gfp_mask;
 }
--- common/inc/nv-linux.h.dist	2016-06-16 04:49:57.775133204 -0400
+++ common/inc/nv-linux.h	2016-06-16 18:36:13.760153738 -0400
@@ -412,12 +412,16 @@ 
 #define __GFP_COMP 0
 #endif

+#if !defined(GFP_USERCOPY)
+#define GPF_USERCOPY 0
+#endif
+
 #if !defined(DEBUG) && defined(__GFP_NOWARN)
-#define NV_GFP_KERNEL (GFP_KERNEL | __GFP_NOWARN)
-#define NV_GFP_ATOMIC (GFP_ATOMIC | __GFP_NOWARN)
+#define NV_GFP_KERNEL (GFP_KERNEL | __GFP_NOWARN | GFP_USERCOPY)
+#define NV_GFP_ATOMIC (GFP_ATOMIC | __GFP_NOWARN | GFP_USERCOPY)
 #else
-#define NV_GFP_KERNEL (GFP_KERNEL)
-#define NV_GFP_ATOMIC (GFP_ATOMIC)
+#define NV_GFP_KERNEL (GFP_KERNEL | GFP_USERCOPY)
+#define NV_GFP_ATOMIC (GFP_ATOMIC | GFP_USERCOPY)
 #endif

 #if defined(GFP_DMA32)
@@ -427,9 +431,9 @@ 
  * such as Linux/x86-64; the alternative is to use an IOMMU such
  * as the one implemented with the K8 GART, if available.
  */
-#define NV_GFP_DMA32 (NV_GFP_KERNEL | GFP_DMA32)
+#define NV_GFP_DMA32 (NV_GFP_KERNEL | GFP_DMA32 | GFP_USERCOPY)
 #else
-#define NV_GFP_DMA32 (NV_GFP_KERNEL)
+#define NV_GFP_DMA32 (NV_GFP_KERNEL | GFP_USERCOPY)
 #endif

 #if defined(NVCPU_X86) || defined(NVCPU_X86_64)
@@ -1307,8 +1311,12 @@ 
     kmem_cache_create(name, size, align, flags, ctor, NULL)
 #endif

+#if !defined(SLAB_USERCOPY)
+#define SLAB_USERCOPY 0
+#endif
+
 #define NV_KMEM_CACHE_CREATE(name, type)    \
-    NV_KMEM_CACHE_CREATE_FULL(name, sizeof(type), 0, 0, NULL)
+    NV_KMEM_CACHE_CREATE_FULL(name, sizeof(type), 0, SLAB_USERCOPY, NULL)

 #define NV_KMEM_CACHE_DESTROY(kmem_cache)   \
     kmem_cache_destroy(kmem_cache)