Hi,
[I'm going to carry this series in my kspp -next tree now, though I'd
really love to have some explicit Acked-bys or Reviewed-bys. If you've
looked through it or tested it, please consider it. :) (I added Valdis
and mpe's Tested-bys where they seemed correct, thank you!)]
This is a start of the mainline port of PAX_USERCOPY[1]. After I started
writing tests (now in lkdtm in -next) for Casey's earlier port[2], I kept
tweaking things further and further until I ended up with a whole new
patch series. To that end, I took Rik and other people's feedback along
with other changes and clean-ups.
Based on my understanding, PAX_USERCOPY was designed to catch a
few classes of flaws (mainly bad bounds checking) around the use of
copy_to_user()/copy_from_user(). These changes don't touch get_user() and
put_user(), since these operate on constant sized lengths, and tend to be
much less vulnerable. There are effectively three distinct protections in
the whole series, each of which I've given a separate CONFIG, though this
patch set is only the first of the three intended protections. (Generally
speaking, PAX_USERCOPY covers what I'm calling CONFIG_HARDENED_USERCOPY
(this) and CONFIG_HARDENED_USERCOPY_WHITELIST (future), and
PAX_USERCOPY_SLABS covers CONFIG_HARDENED_USERCOPY_SPLIT_KMALLOC
(future).)
This series, which adds CONFIG_HARDENED_USERCOPY, checks that objects
being copied to/from userspace meet certain criteria:
- if address is a heap object, the size must not exceed the object's
allocated size. (This will catch all kinds of heap overflow flaws.)
- if address range is in the current process stack, it must be within the
current stack frame (if such checking is possible) or at least entirely
within the current process's stack. (This could catch large lengths that
would have extended beyond the current process stack, or overflows if
their length extends back into the original stack.)
- if the address range is part of kernel data, rodata, or bss, allow it.
- if address range is page-allocated, that it doesn't span multiple
allocations.
- if address is within the kernel text, reject it.
- everything else is accepted
The patches in the series are:
- Support for arch-specific stack frame checking (which will likely be
replaced in the future by Josh's more comprehensive unwinder):
1- mm: Implement stack frame object validation
- The core copy_to/from_user() checks, without the slab object checks:
2- mm: Hardened usercopy
- Per-arch enablement of the protection:
3- x86/uaccess: Enable hardened usercopy
4- ARM: uaccess: Enable hardened usercopy
5- arm64/uaccess: Enable hardened usercopy
6- ia64/uaccess: Enable hardened usercopy
7- powerpc/uaccess: Enable hardened usercopy
8- sparc/uaccess: Enable hardened usercopy
9- s390/uaccess: Enable hardened usercopy
- The heap allocator implementation of object size checking:
10- mm: SLAB hardened usercopy support
11- mm: SLUB hardened usercopy support
Some notes:
- This is expected to apply on top of -next which contains fixes for the
position of _etext on both arm and arm64, though it has minor conflicts
with KASAN that are trivial to fix up. Living in -next are also tests
for this protection in lkdtm, prefixed with USERCOPY_.
- I couldn't detect a measurable performance change with these features
enabled. Kernel build times were unchanged, hackbench was unchanged,
etc. I think we could flip this to "on by default" at some point, but
for now, I'm leaving it off until I can get some more definitive
measurements. I would love if someone with greater familiarity with
perf could give this a spin and report results.
- The SLOB support extracted from grsecurity seems entirely broken. I
have no idea what's going on there, I spent my time testing SLAB and
SLUB. Having someone else look at SLOB would be nice, but this series
doesn't depend on it.
Additional features that would be nice, but aren't blocking this series:
- Needs more architecture support for stack frame checking (only x86 now,
but it seems Josh will have a good solution for this soon).
Thanks!
-Kees
[1] https://grsecurity.net/download.php "grsecurity - test kernel patch"
[2] http://www.openwall.com/lists/kernel-hardening/2016/05/19/5
v3:
- switch to using BUG for better Oops integration
- when checking page allocations, check each for Reserved
- use enums for the stack check return for readability
v2:
- added s390 support
- handle slub red zone
- disallow writes to rodata area
- stack frame walker now CONFIG-controlled arch-specific helper
This creates per-architecture function arch_within_stack_frames() that
should validate if a given object is contained by a kernel stack frame.
Initial implementation is on x86.
This is based on code from PaX.
Signed-off-by: Kees Cook <redacted>
---
arch/Kconfig | 9 ++++++++
arch/x86/Kconfig | 1 +
arch/x86/include/asm/thread_info.h | 44 ++++++++++++++++++++++++++++++++++++++
include/linux/thread_info.h | 9 ++++++++
4 files changed, 63 insertions(+)
Enables CONFIG_HARDENED_USERCOPY checks on x86. This is done both in
copy_*_user() and __copy_*_user() because copy_*_user() actually calls
down to _copy_*_user() and not __copy_*_user().
Based on code from PaX and grsecurity.
Signed-off-by: Kees Cook <redacted>
Tested-By: Valdis Kletnieks <redacted>
---
arch/x86/Kconfig | 2 ++
arch/x86/include/asm/uaccess.h | 10 ++++++----
arch/x86/include/asm/uaccess_32.h | 2 ++
arch/x86/include/asm/uaccess_64.h | 2 ++
4 files changed, 12 insertions(+), 4 deletions(-)
@@ -762,9 +763,10 @@ copy_to_user(void __user *to, const void *from, unsigned long n)might_fault();/* See the comment in copy_from_user() above. */-if(likely(sz<0||sz>=n))+if(likely(sz<0||sz>=n)){+check_object_size(from,n,true);n=_copy_to_user(to,from,n);-elseif(__builtin_constant_p(n))+}elseif(__builtin_constant_p(n))copy_to_user_overflow();else__copy_to_user_overflow(sz,n);
Under CONFIG_HARDENED_USERCOPY, this adds object size checking to the
SLAB allocator to catch any copies that may span objects.
Based on code from PaX and grsecurity.
Signed-off-by: Kees Cook <redacted>
Tested-By: Valdis Kletnieks <redacted>
---
init/Kconfig | 1 +
mm/slab.c | 30 ++++++++++++++++++++++++++++++
2 files changed, 31 insertions(+)
Under CONFIG_HARDENED_USERCOPY, this adds object size checking to the
SLUB allocator to catch any copies that may span objects. Includes a
redzone handling fix discovered by Michael Ellerman.
Based on code from PaX and grsecurity.
Signed-off-by: Kees Cook <redacted>
Tested-by: Michael Ellerman <mpe@ellerman.id.au>
---
init/Kconfig | 1 +
mm/slub.c | 36 ++++++++++++++++++++++++++++++++++++
2 files changed, 37 insertions(+)
@@ -3614,6 +3614,42 @@ void *__kmalloc_node(size_t size, gfp_t flags, int node)EXPORT_SYMBOL(__kmalloc_node);#endif+#ifdef CONFIG_HARDENED_USERCOPY+/*+*Rejectsobjectsthatareincorrectlysized.+*+*ReturnsNULLifcheckpasses,otherwiseconstchar*tonameofcache+*toindicateanerror.+*/+constchar*__check_heap_object(constvoid*ptr,unsignedlongn,+structpage*page)+{+structkmem_cache*s;+unsignedlongoffset;+size_tobject_size;++/* Find object and usable object size. */+s=page->slab_cache;+object_size=slab_ksize(s);++/* Find offset within object. */+offset=(ptr-page_address(page))%s->size;++/* Adjust for redzone and reject if within the redzone. */+if(kmem_cache_debug(s)&&s->flags&SLAB_RED_ZONE){+if(offset<s->red_left_pad)+returns->name;+offset-=s->red_left_pad;+}++/* Allow address range falling entirely within object size. */+if(offset<=object_size&&n<=object_size-offset)+returnNULL;++returns->name;+}+#endif /* CONFIG_HARDENED_USERCOPY */+staticsize_t__ksize(constvoid*object){structpage*page;
Enables CONFIG_HARDENED_USERCOPY checks on arm64. As done by KASAN in -next,
renames the low-level functions to __arch_copy_*_user() so a static inline
can do additional work before the copy.
Signed-off-by: Kees Cook <redacted>
---
arch/arm64/Kconfig | 2 ++
arch/arm64/include/asm/uaccess.h | 16 ++++++++++++++--
arch/arm64/kernel/arm64ksyms.c | 4 ++--
arch/arm64/lib/copy_from_user.S | 4 ++--
arch/arm64/lib/copy_to_user.S | 4 ++--
5 files changed, 22 insertions(+), 8 deletions(-)
@@ -34,8 +34,8 @@ EXPORT_SYMBOL(copy_page);EXPORT_SYMBOL(clear_page);/* user mem (segment) */-EXPORT_SYMBOL(__copy_from_user);-EXPORT_SYMBOL(__copy_to_user);+EXPORT_SYMBOL(__arch_copy_from_user);+EXPORT_SYMBOL(__arch_copy_to_user);EXPORT_SYMBOL(__clear_user);EXPORT_SYMBOL(__copy_in_user);
This is the start of porting PAX_USERCOPY into the mainline kernel. This
is the first set of features, controlled by CONFIG_HARDENED_USERCOPY. The
work is based on code by PaX Team and Brad Spengler, and an earlier port
from Casey Schaufler. Additional non-slab page tests are from Rik van Riel.
This patch contains the logic for validating several conditions when
performing copy_to_user() and copy_from_user() on the kernel object
being copied to/from:
- address range doesn't wrap around
- address range isn't NULL or zero-allocated (with a non-zero copy size)
- if on the slab allocator:
- object size must be less than or equal to copy size (when check is
implemented in the allocator, which appear in subsequent patches)
- otherwise, object must not span page allocations
- if on the stack
- object must not extend before/after the current process task
- object must be contained by the current stack frame (when there is
arch/build support for identifying stack frames)
- object must not overlap with kernel text
Signed-off-by: Kees Cook <redacted>
Tested-By: Valdis Kletnieks <redacted>
Tested-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/Kconfig | 7 ++
include/linux/slab.h | 12 +++
include/linux/thread_info.h | 15 +++
mm/Makefile | 4 +
mm/usercopy.c | 234 ++++++++++++++++++++++++++++++++++++++++++++
security/Kconfig | 28 ++++++
6 files changed, 300 insertions(+)
create mode 100644 mm/usercopy.c
@@ -21,6 +21,9 @@ KCOV_INSTRUMENT_memcontrol.o := nKCOV_INSTRUMENT_mmzone.o:=nKCOV_INSTRUMENT_vmstat.o:=n+# Since __builtin_frame_address does work as used, disable the warning.+CFLAGS_usercopy.o+=$(callcc-disable-warning,frame-address)+mmu-y:=nommu.ommu-$(CONFIG_MMU):=gup.ohighmem.omemory.omincore.o\mlock.ommap.omprotect.omremap.omsync.ormap.o\
@@ -0,0 +1,234 @@+/*+*ThisimplementsthevariouschecksforCONFIG_HARDENED_USERCOPY*,+*whicharedesignedtoprotectkernelmemoryfromneedlessexposure+*andoverwriteundermanyunintendedconditions.Thiscodeisbased+*onPAX_USERCOPY,whichis:+*+*Copyright(C)2001-2016PaXTeam,BradleySpengler,OpenSource+*SecurityInc.+*+*Thisprogramisfreesoftware;youcanredistributeitand/ormodify+*itunderthetermsoftheGNUGeneralPublicLicenseversion2as+*publishedbytheFreeSoftwareFoundation.+*+*/+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt++#include<linux/mm.h>+#include<linux/slab.h>+#include<asm/sections.h>++enum{+BAD_STACK=-1,+NOT_STACK=0,+GOOD_FRAME,+GOOD_STACK,+};++/*+*Checksifagivenpointerandlengthiscontainedbythecurrent+*stackframe(ifpossible).+*+*0:notatallonthestack+*1:fullywithinavalidstackframe+*2:fullyonthestack(whencan'tdoframe-checking)+*-1:errorcondition(invalidstackpositionorbadstackframe)+*/+staticnoinlineintcheck_stack_object(constvoid*obj,unsignedlonglen)+{+constvoid*conststack=task_stack_page(current);+constvoid*conststackend=stack+THREAD_SIZE;+intret;++/* Object is not on the stack at all. */+if(obj+len<=stack||stackend<=obj)+returnNOT_STACK;++/*+*Reject:objectpartiallyoverlapsthestack(passingthe+*thecheckabovemeansatleastoneendiswithinthestack,+*soifthischeckfails,theotherendisoutsidethestack).+*/+if(obj<stack||stackend<obj+len)+returnBAD_STACK;++/* Check if object is safely within a valid frame. */+ret=arch_within_stack_frames(stack,stackend,obj,len);+if(ret)+returnret;++returnGOOD_STACK;+}++staticvoidreport_usercopy(constvoid*ptr,unsignedlonglen,+boolto_user,constchar*type)+{+pr_emerg("kernel memory %s attempt detected %s %p (%s) (%lu bytes)\n",+to_user?"exposure":"overwrite",+to_user?"from":"to",ptr,type?:"unknown",len);+/*+*Forgreatereffect,itwouldbenicetododo_group_exit(),+*butBUG()actuallyhooksallthelock-breakingandper-arch+*Oopscode,sothatisusedhereinstead.+*/+BUG();+}++/* Returns true if any portion of [ptr,ptr+n) over laps with [low,high). */+staticbooloverlaps(constvoid*ptr,unsignedlongn,unsignedlonglow,+unsignedlonghigh)+{+unsignedlongcheck_low=(uintptr_t)ptr;+unsignedlongcheck_high=check_low+n;++/* Does not overlap if entirely above or entirely below. */+if(check_low>=high||check_high<low)+returnfalse;++returntrue;+}++/* Is this address range in the kernel text area? */+staticinlineconstchar*check_kernel_text_object(constvoid*ptr,+unsignedlongn)+{+unsignedlongtextlow=(unsignedlong)_stext;+unsignedlongtexthigh=(unsignedlong)_etext;++if(overlaps(ptr,n,textlow,texthigh))+return"<kernel text>";++#ifdef HAVE_ARCH_LINEAR_KERNEL_MAPPING+/* Check against linear mapping as well. */+if(overlaps(ptr,n,(unsignedlong)__va(__pa(textlow)),+(unsignedlong)__va(__pa(texthigh))))+return"<linear kernel text>";+#endif++returnNULL;+}++staticinlineconstchar*check_bogus_address(constvoid*ptr,unsignedlongn)+{+/* Reject if object wraps past end of memory. */+if(ptr+n<ptr)+return"<wrapped address>";++/* Reject if NULL or ZERO-allocation. */+if(ZERO_OR_NULL_PTR(ptr))+return"<null>";++returnNULL;+}++staticinlineconstchar*check_heap_object(constvoid*ptr,unsignedlongn,+boolto_user)+{+structpage*page,*endpage;+constvoid*end=ptr+n-1;++if(!virt_addr_valid(ptr))+returnNULL;++page=virt_to_head_page(ptr);++/* Check slab allocator for flags and size. */+if(PageSlab(page))+return__check_heap_object(ptr,n,page);++/*+*SometimesthekerneldataregionsarenotmarkedReserved(see+*checkbelow).Andsometimes[_sdata,_edata)doesnotcover+*rodataand/orbss,socheckeachrangeexplicitly.+*/++/* Allow reads of kernel rodata region (if not marked as Reserved). */+if(ptr>=(constvoid*)__start_rodata&&+end<=(constvoid*)__end_rodata){+if(!to_user)+return"<rodata>";+returnNULL;+}++/* Allow kernel data region (if not marked as Reserved). */+if(ptr>=(constvoid*)_sdata&&end<=(constvoid*)_edata)+returnNULL;++/* Allow kernel bss region (if not marked as Reserved). */+if(ptr>=(constvoid*)__bss_start&&+end<=(constvoid*)__bss_stop)+returnNULL;++/* Is the object wholly within one base page? */+if(likely(((unsignedlong)ptr&(unsignedlong)PAGE_MASK)==+((unsignedlong)end&(unsignedlong)PAGE_MASK)))+returnNULL;++/* Allow if start and end are inside the same compound page. */+endpage=virt_to_head_page(end);+if(likely(endpage==page))+returnNULL;++/*+*RejectifrangeisnotReserved(i.e.specialordevicememory),+*sincethentheobjectspansseveralindependentlyallocatedpages.+*/+for(;ptr<=end;ptr+=PAGE_SIZE,page=virt_to_head_page(ptr)){+if(!PageReserved(page))+return"<spans multiple pages>";+}++returnNULL;+}++/*+*Validatesthatthegivenobjectisoneof:+*-knownsafeheapobject+*-knownsafestackobject+*-notinkerneltext+*/+void__check_object_size(constvoid*ptr,unsignedlongn,boolto_user)+{+constchar*err;++/* Skip all tests if size is zero. */+if(!n)+return;++/* Check for invalid addresses. */+err=check_bogus_address(ptr,n);+if(err)+gotoreport;++/* Check for bad heap object. */+err=check_heap_object(ptr,n,to_user);+if(err)+gotoreport;++/* Check for bad stack object. */+switch(check_stack_object(ptr,n)){+caseNOT_STACK:+/* Object is not touching the current process stack. */+break;+caseGOOD_FRAME:+caseGOOD_STACK:+/*+*Objectiseitherinthecorrectframe(whenit+*ispossibletocheck)orjustgenerallyonthe+*processstack(whenframecheckingnotavailable).+*/+return;+default:+err="<process stack>";+gotoreport;+}++/* Check for object in kernel to avoid text exposure. */+err=check_kernel_text_object(ptr,n);+if(!err)+return;++report:+report_usercopy(ptr,n,to_user,err);+}+EXPORT_SYMBOL(__check_object_size);
On Fri, 2016-07-15 at 14:44 -0700, Kees Cook wrote:
Hi,
[I'm going to carry this series in my kspp -next tree now, though I'd
really love to have some explicit Acked-bys or Reviewed-bys. If you've
looked through it or tested it, please consider it. :) (I added Valdis
and mpe's Tested-bys where they seemed correct, thank you!)]
This is a start of the mainline port of PAX_USERCOPY[1]. After I started
writing tests (now in lkdtm in -next) for Casey's earlier port[2], I kept
tweaking things further and further until I ended up with a whole new
patch series. To that end, I took Rik and other people's feedback along
with other changes and clean-ups.
Based on my understanding, PAX_USERCOPY was designed to catch a
few classes of flaws (mainly bad bounds checking) around the use of
copy_to_user()/copy_from_user(). These changes don't touch get_user() and
put_user(), since these operate on constant sized lengths, and tend to be
much less vulnerable. There are effectively three distinct protections in
the whole series, each of which I've given a separate CONFIG, though this
patch set is only the first of the three intended protections. (Generally
speaking, PAX_USERCOPY covers what I'm calling CONFIG_HARDENED_USERCOPY
(this) and CONFIG_HARDENED_USERCOPY_WHITELIST (future), and
PAX_USERCOPY_SLABS covers CONFIG_HARDENED_USERCOPY_SPLIT_KMALLOC
(future).)
This series, which adds CONFIG_HARDENED_USERCOPY, checks that objects
being copied to/from userspace meet certain criteria:
- if address is a heap object, the size must not exceed the object's
allocated size. (This will catch all kinds of heap overflow flaws.)
- if address range is in the current process stack, it must be within the
current stack frame (if such checking is possible) or at least entirely
within the current process's stack. (This could catch large lengths that
would have extended beyond the current process stack, or overflows if
their length extends back into the original stack.)
- if the address range is part of kernel data, rodata, or bss, allow it.
- if address range is page-allocated, that it doesn't span multiple
allocations.
- if address is within the kernel text, reject it.
- everything else is accepted
The patches in the series are:
- Support for arch-specific stack frame checking (which will likely be
replaced in the future by Josh's more comprehensive unwinder):
1- mm: Implement stack frame object validation
- The core copy_to/from_user() checks, without the slab object checks:
2- mm: Hardened usercopy
- Per-arch enablement of the protection:
3- x86/uaccess: Enable hardened usercopy
4- ARM: uaccess: Enable hardened usercopy
5- arm64/uaccess: Enable hardened usercopy
6- ia64/uaccess: Enable hardened usercopy
7- powerpc/uaccess: Enable hardened usercopy
8- sparc/uaccess: Enable hardened usercopy
9- s390/uaccess: Enable hardened usercopy
- The heap allocator implementation of object size checking:
10- mm: SLAB hardened usercopy support
11- mm: SLUB hardened usercopy support
Some notes:
- This is expected to apply on top of -next which contains fixes for the
position of _etext on both arm and arm64, though it has minor conflicts
with KASAN that are trivial to fix up. Living in -next are also tests
for this protection in lkdtm, prefixed with USERCOPY_.
- I couldn't detect a measurable performance change with these features
enabled. Kernel build times were unchanged, hackbench was unchanged,
etc. I think we could flip this to "on by default" at some point, but
for now, I'm leaving it off until I can get some more definitive
measurements. I would love if someone with greater familiarity with
perf could give this a spin and report results.
- The SLOB support extracted from grsecurity seems entirely broken. I
have no idea what's going on there, I spent my time testing SLAB and
SLUB. Having someone else look at SLOB would be nice, but this series
doesn't depend on it.
Additional features that would be nice, but aren't blocking this series:
- Needs more architecture support for stack frame checking (only x86 now,
but it seems Josh will have a good solution for this soon).
Thanks!
-Kees
[1] https://grsecurity.net/download.php "grsecurity - test kernel patch"
[2] http://www.openwall.com/lists/kernel-hardening/2016/05/19/5
v3:
- switch to using BUG for better Oops integration
- when checking page allocations, check each for Reserved
- use enums for the stack check return for readability
Thanks looks good so far! I'll try and test it and report back
Balbir
From: Laura Abbott <hidden> Date: 2016-07-19 01:06:45
On 07/15/2016 02:44 PM, Kees Cook wrote:
quoted hunk
This is the start of porting PAX_USERCOPY into the mainline kernel. This
is the first set of features, controlled by CONFIG_HARDENED_USERCOPY. The
work is based on code by PaX Team and Brad Spengler, and an earlier port
from Casey Schaufler. Additional non-slab page tests are from Rik van Riel.
This patch contains the logic for validating several conditions when
performing copy_to_user() and copy_from_user() on the kernel object
being copied to/from:
- address range doesn't wrap around
- address range isn't NULL or zero-allocated (with a non-zero copy size)
- if on the slab allocator:
- object size must be less than or equal to copy size (when check is
implemented in the allocator, which appear in subsequent patches)
- otherwise, object must not span page allocations
- if on the stack
- object must not extend before/after the current process task
- object must be contained by the current stack frame (when there is
arch/build support for identifying stack frames)
- object must not overlap with kernel text
Signed-off-by: Kees Cook <redacted>
Tested-By: Valdis Kletnieks <redacted>
Tested-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/Kconfig | 7 ++
include/linux/slab.h | 12 +++
include/linux/thread_info.h | 15 +++
mm/Makefile | 4 +
mm/usercopy.c | 234 ++++++++++++++++++++++++++++++++++++++++++++
security/Kconfig | 28 ++++++
6 files changed, 300 insertions(+)
create mode 100644 mm/usercopy.c
@@ -21,6 +21,9 @@ KCOV_INSTRUMENT_memcontrol.o := nKCOV_INSTRUMENT_mmzone.o:=nKCOV_INSTRUMENT_vmstat.o:=n+# Since __builtin_frame_address does work as used, disable the warning.+CFLAGS_usercopy.o+=$(callcc-disable-warning,frame-address)+mmu-y:=nommu.ommu-$(CONFIG_MMU):=gup.ohighmem.omemory.omincore.o\mlock.ommap.omprotect.omremap.omsync.ormap.o\
Nit: update comments to match enum (BAD_STACK instead of -1 etc.)
+static noinline int check_stack_object(const void *obj, unsigned long len)
+{
+ const void * const stack = task_stack_page(current);
+ const void * const stackend = stack + THREAD_SIZE;
+ int ret;
+
+ /* Object is not on the stack at all. */
+ if (obj + len <= stack || stackend <= obj)
+ return NOT_STACK;
+
+ /*
+ * Reject: object partially overlaps the stack (passing the
+ * the check above means at least one end is within the stack,
+ * so if this check fails, the other end is outside the stack).
+ */
+ if (obj < stack || stackend < obj + len)
+ return BAD_STACK;
+
+ /* Check if object is safely within a valid frame. */
+ ret = arch_within_stack_frames(stack, stackend, obj, len);
+ if (ret)
+ return ret;
+
+ return GOOD_STACK;
+}
+
+static void report_usercopy(const void *ptr, unsigned long len,
+ bool to_user, const char *type)
+{
+ pr_emerg("kernel memory %s attempt detected %s %p (%s) (%lu bytes)\n",
+ to_user ? "exposure" : "overwrite",
+ to_user ? "from" : "to", ptr, type ? : "unknown", len);
+ /*
+ * For greater effect, it would be nice to do do_group_exit(),
+ * but BUG() actually hooks all the lock-breaking and per-arch
+ * Oops code, so that is used here instead.
+ */
+ BUG();
+}
+
+/* Returns true if any portion of [ptr,ptr+n) over laps with [low,high). */
+static bool overlaps(const void *ptr, unsigned long n, unsigned long low,
+ unsigned long high)
+{
+ unsigned long check_low = (uintptr_t)ptr;
+ unsigned long check_high = check_low + n;
+
+ /* Does not overlap if entirely above or entirely below. */
+ if (check_low >= high || check_high < low)
+ return false;
+
+ return true;
+}
+
+/* Is this address range in the kernel text area? */
+static inline const char *check_kernel_text_object(const void *ptr,
+ unsigned long n)
+{
+ unsigned long textlow = (unsigned long)_stext;
+ unsigned long texthigh = (unsigned long)_etext;
+
+ if (overlaps(ptr, n, textlow, texthigh))
+ return "<kernel text>";
+
+#ifdef HAVE_ARCH_LINEAR_KERNEL_MAPPING
+ /* Check against linear mapping as well. */
+ if (overlaps(ptr, n, (unsigned long)__va(__pa(textlow)),
+ (unsigned long)__va(__pa(texthigh))))
+ return "<linear kernel text>";
+#endif
+
+ return NULL;
+}
+
+static inline const char *check_bogus_address(const void *ptr, unsigned long n)
+{
+ /* Reject if object wraps past end of memory. */
+ if (ptr + n < ptr)
+ return "<wrapped address>";
+
+ /* Reject if NULL or ZERO-allocation. */
+ if (ZERO_OR_NULL_PTR(ptr))
+ return "<null>";
+
+ return NULL;
+}
+
+static inline const char *check_heap_object(const void *ptr, unsigned long n,
+ bool to_user)
+{
+ struct page *page, *endpage;
+ const void *end = ptr + n - 1;
+
+ if (!virt_addr_valid(ptr))
+ return NULL;
+
+ page = virt_to_head_page(ptr);
+
+ /* Check slab allocator for flags and size. */
+ if (PageSlab(page))
+ return __check_heap_object(ptr, n, page);
+
+ /*
+ * Sometimes the kernel data regions are not marked Reserved (see
+ * check below). And sometimes [_sdata,_edata) does not cover
+ * rodata and/or bss, so check each range explicitly.
+ */
+
+ /* Allow reads of kernel rodata region (if not marked as Reserved). */
+ if (ptr >= (const void *)__start_rodata &&
+ end <= (const void *)__end_rodata) {
+ if (!to_user)
+ return "<rodata>";
+ return NULL;
+ }
+
+ /* Allow kernel data region (if not marked as Reserved). */
+ if (ptr >= (const void *)_sdata && end <= (const void *)_edata)
+ return NULL;
+
+ /* Allow kernel bss region (if not marked as Reserved). */
+ if (ptr >= (const void *)__bss_start &&
+ end <= (const void *)__bss_stop)
+ return NULL;
+
+ /* Is the object wholly within one base page? */
+ if (likely(((unsigned long)ptr & (unsigned long)PAGE_MASK) ==
+ ((unsigned long)end & (unsigned long)PAGE_MASK)))
+ return NULL;
+
+ /* Allow if start and end are inside the same compound page. */
+ endpage = virt_to_head_page(end);
+ if (likely(endpage == page))
+ return NULL;
+
+ /*
+ * Reject if range is not Reserved (i.e. special or device memory),
+ * since then the object spans several independently allocated pages.
+ */
+ for (; ptr <= end ; ptr += PAGE_SIZE, page = virt_to_head_page(ptr)) {
+ if (!PageReserved(page))
+ return "<spans multiple pages>";
+ }
+
This doesn't work when copying CMA allocated memory since CMA purposely
allocates larger than a page block size without setting head pages.
Given CMA may be used with drivers doing zero copy buffers, I think it
should be permitted.
Something like the following lets it pass (I can clean up and submit
the is_migrate_cma_page APIs as a separate patch for review)
From: Laura Abbott <hidden> Date: 2016-07-19 01:52:35
On 07/15/2016 02:44 PM, Kees Cook wrote:
quoted hunk
This is the start of porting PAX_USERCOPY into the mainline kernel. This
is the first set of features, controlled by CONFIG_HARDENED_USERCOPY. The
work is based on code by PaX Team and Brad Spengler, and an earlier port
from Casey Schaufler. Additional non-slab page tests are from Rik van Riel.
This patch contains the logic for validating several conditions when
performing copy_to_user() and copy_from_user() on the kernel object
being copied to/from:
- address range doesn't wrap around
- address range isn't NULL or zero-allocated (with a non-zero copy size)
- if on the slab allocator:
- object size must be less than or equal to copy size (when check is
implemented in the allocator, which appear in subsequent patches)
- otherwise, object must not span page allocations
- if on the stack
- object must not extend before/after the current process task
- object must be contained by the current stack frame (when there is
arch/build support for identifying stack frames)
- object must not overlap with kernel text
Signed-off-by: Kees Cook <redacted>
Tested-By: Valdis Kletnieks <redacted>
Tested-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/Kconfig | 7 ++
include/linux/slab.h | 12 +++
include/linux/thread_info.h | 15 +++
mm/Makefile | 4 +
mm/usercopy.c | 234 ++++++++++++++++++++++++++++++++++++++++++++
security/Kconfig | 28 ++++++
6 files changed, 300 insertions(+)
create mode 100644 mm/usercopy.c
@@ -21,6 +21,9 @@ KCOV_INSTRUMENT_memcontrol.o := nKCOV_INSTRUMENT_mmzone.o:=nKCOV_INSTRUMENT_vmstat.o:=n+# Since __builtin_frame_address does work as used, disable the warning.+CFLAGS_usercopy.o+=$(callcc-disable-warning,frame-address)+mmu-y:=nommu.ommu-$(CONFIG_MMU):=gup.ohighmem.omemory.omincore.o\mlock.ommap.omprotect.omremap.omsync.ormap.o\
@@ -0,0 +1,234 @@+/*+*ThisimplementsthevariouschecksforCONFIG_HARDENED_USERCOPY*,+*whicharedesignedtoprotectkernelmemoryfromneedlessexposure+*andoverwriteundermanyunintendedconditions.Thiscodeisbased+*onPAX_USERCOPY,whichis:+*+*Copyright(C)2001-2016PaXTeam,BradleySpengler,OpenSource+*SecurityInc.+*+*Thisprogramisfreesoftware;youcanredistributeitand/ormodify+*itunderthetermsoftheGNUGeneralPublicLicenseversion2as+*publishedbytheFreeSoftwareFoundation.+*+*/+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt++#include<linux/mm.h>+#include<linux/slab.h>+#include<asm/sections.h>++enum{+BAD_STACK=-1,+NOT_STACK=0,+GOOD_FRAME,+GOOD_STACK,+};++/*+*Checksifagivenpointerandlengthiscontainedbythecurrent+*stackframe(ifpossible).+*+*0:notatallonthestack+*1:fullywithinavalidstackframe+*2:fullyonthestack(whencan'tdoframe-checking)+*-1:errorcondition(invalidstackpositionorbadstackframe)+*/+staticnoinlineintcheck_stack_object(constvoid*obj,unsignedlonglen)+{+constvoid*conststack=task_stack_page(current);+constvoid*conststackend=stack+THREAD_SIZE;+intret;++/* Object is not on the stack at all. */+if(obj+len<=stack||stackend<=obj)+returnNOT_STACK;++/*+*Reject:objectpartiallyoverlapsthestack(passingthe+*thecheckabovemeansatleastoneendiswithinthestack,+*soifthischeckfails,theotherendisoutsidethestack).+*/+if(obj<stack||stackend<obj+len)+returnBAD_STACK;++/* Check if object is safely within a valid frame. */+ret=arch_within_stack_frames(stack,stackend,obj,len);+if(ret)+returnret;++returnGOOD_STACK;+}++staticvoidreport_usercopy(constvoid*ptr,unsignedlonglen,+boolto_user,constchar*type)+{+pr_emerg("kernel memory %s attempt detected %s %p (%s) (%lu bytes)\n",+to_user?"exposure":"overwrite",+to_user?"from":"to",ptr,type?:"unknown",len);+/*+*Forgreatereffect,itwouldbenicetododo_group_exit(),+*butBUG()actuallyhooksallthelock-breakingandper-arch+*Oopscode,sothatisusedhereinstead.+*/+BUG();+}++/* Returns true if any portion of [ptr,ptr+n) over laps with [low,high). */+staticbooloverlaps(constvoid*ptr,unsignedlongn,unsignedlonglow,+unsignedlonghigh)+{+unsignedlongcheck_low=(uintptr_t)ptr;+unsignedlongcheck_high=check_low+n;++/* Does not overlap if entirely above or entirely below. */+if(check_low>=high||check_high<low)+returnfalse;++returntrue;+}++/* Is this address range in the kernel text area? */+staticinlineconstchar*check_kernel_text_object(constvoid*ptr,+unsignedlongn)+{+unsignedlongtextlow=(unsignedlong)_stext;+unsignedlongtexthigh=(unsignedlong)_etext;++if(overlaps(ptr,n,textlow,texthigh))+return"<kernel text>";++#ifdef HAVE_ARCH_LINEAR_KERNEL_MAPPING+/* Check against linear mapping as well. */+if(overlaps(ptr,n,(unsignedlong)__va(__pa(textlow)),+(unsignedlong)__va(__pa(texthigh))))+return"<linear kernel text>";+#endif++returnNULL;+}++staticinlineconstchar*check_bogus_address(constvoid*ptr,unsignedlongn)+{+/* Reject if object wraps past end of memory. */+if(ptr+n<ptr)+return"<wrapped address>";++/* Reject if NULL or ZERO-allocation. */+if(ZERO_OR_NULL_PTR(ptr))+return"<null>";++returnNULL;+}++staticinlineconstchar*check_heap_object(constvoid*ptr,unsignedlongn,+boolto_user)+{+structpage*page,*endpage;+constvoid*end=ptr+n-1;++if(!virt_addr_valid(ptr))+returnNULL;+
virt_addr_valid returns true on vmalloc addresses on arm64 which causes some
intermittent false positives (tab completion in a qemu buildroot environment
was showing it fairly reliably). I think this is an arm64 bug because
virt_addr_valid should return true if and only if virt_to_page returns the
corresponding page. We can work around this for now by explicitly
checking against is_vmalloc_addr.
Thanks,
Laura
quoted hunk
+ page = virt_to_head_page(ptr);
+
+ /* Check slab allocator for flags and size. */
+ if (PageSlab(page))
+ return __check_heap_object(ptr, n, page);
+
+ /*
+ * Sometimes the kernel data regions are not marked Reserved (see
+ * check below). And sometimes [_sdata,_edata) does not cover
+ * rodata and/or bss, so check each range explicitly.
+ */
+
+ /* Allow reads of kernel rodata region (if not marked as Reserved). */
+ if (ptr >= (const void *)__start_rodata &&
+ end <= (const void *)__end_rodata) {
+ if (!to_user)
+ return "<rodata>";
+ return NULL;
+ }
+
+ /* Allow kernel data region (if not marked as Reserved). */
+ if (ptr >= (const void *)_sdata && end <= (const void *)_edata)
+ return NULL;
+
+ /* Allow kernel bss region (if not marked as Reserved). */
+ if (ptr >= (const void *)__bss_start &&
+ end <= (const void *)__bss_stop)
+ return NULL;
+
+ /* Is the object wholly within one base page? */
+ if (likely(((unsigned long)ptr & (unsigned long)PAGE_MASK) ==
+ ((unsigned long)end & (unsigned long)PAGE_MASK)))
+ return NULL;
+
+ /* Allow if start and end are inside the same compound page. */
+ endpage = virt_to_head_page(end);
+ if (likely(endpage == page))
+ return NULL;
+
+ /*
+ * Reject if range is not Reserved (i.e. special or device memory),
+ * since then the object spans several independently allocated pages.
+ */
+ for (; ptr <= end ; ptr += PAGE_SIZE, page = virt_to_head_page(ptr)) {
+ if (!PageReserved(page))
+ return "<spans multiple pages>";
+ }
+
+ return NULL;
+}
+
+/*
+ * Validates that the given object is one of:
+ * - known safe heap object
+ * - known safe stack object
+ * - not in kernel text
+ */
+void __check_object_size(const void *ptr, unsigned long n, bool to_user)
+{
+ const char *err;
+
+ /* Skip all tests if size is zero. */
+ if (!n)
+ return;
+
+ /* Check for invalid addresses. */
+ err = check_bogus_address(ptr, n);
+ if (err)
+ goto report;
+
+ /* Check for bad heap object. */
+ err = check_heap_object(ptr, n, to_user);
+ if (err)
+ goto report;
+
+ /* Check for bad stack object. */
+ switch (check_stack_object(ptr, n)) {
+ case NOT_STACK:
+ /* Object is not touching the current process stack. */
+ break;
+ case GOOD_FRAME:
+ case GOOD_STACK:
+ /*
+ * Object is either in the correct frame (when it
+ * is possible to check) or just generally on the
+ * process stack (when frame checking not available).
+ */
+ return;
+ default:
+ err = "<process stack>";
+ goto report;
+ }
+
+ /* Check for object in kernel to avoid text exposure. */
+ err = check_kernel_text_object(ptr, n);
+ if (!err)
+ return;
+
+report:
+ report_usercopy(ptr, n, to_user, err);
+}
+EXPORT_SYMBOL(__check_object_size);
From: Christian Borntraeger <hidden> Date: 2016-07-19 09:22:11
On 07/15/2016 11:44 PM, Kees Cook wrote:
+config HAVE_ARCH_LINEAR_KERNEL_MAPPING
+ bool
+ help
+ An architecture should select this if it has a secondary linear
+ mapping of the kernel text. This is used to verify that kernel
+ text exposures are not visible under CONFIG_HARDENED_USERCOPY.
I have trouble parsing this. (What does secondary linear mapping mean?)
So let me give an example below
+
[...]
+/* Is this address range in the kernel text area? */
+static inline const char *check_kernel_text_object(const void *ptr,
+ unsigned long n)
+{
+ unsigned long textlow = (unsigned long)_stext;
+ unsigned long texthigh = (unsigned long)_etext;
+
+ if (overlaps(ptr, n, textlow, texthigh))
+ return "<kernel text>";
+
+#ifdef HAVE_ARCH_LINEAR_KERNEL_MAPPING
+ /* Check against linear mapping as well. */
+ if (overlaps(ptr, n, (unsigned long)__va(__pa(textlow)),
+ (unsigned long)__va(__pa(texthigh))))
+ return "<linear kernel text>";
+#endif
+
+ return NULL;
+}
s390 has an address space for user (primary address space from 0..4TB/8PB) and a separate
address space (home space from 0..4TB/8PB) for the kernel. In this home space the kernel
mapping is virtual containing the physical memory as well as vmalloc memory (creating aliases
into the physical one). The kernel text is mapped from _stext to _etext in this mapping.
So I assume this would qualify for HAVE_ARCH_LINEAR_KERNEL_MAPPING ?
On Mon, Jul 18, 2016 at 6:06 PM, Laura Abbott [off-list ref] wrote:
On 07/15/2016 02:44 PM, Kees Cook wrote:
quoted
This is the start of porting PAX_USERCOPY into the mainline kernel. This
is the first set of features, controlled by CONFIG_HARDENED_USERCOPY. The
work is based on code by PaX Team and Brad Spengler, and an earlier port
from Casey Schaufler. Additional non-slab page tests are from Rik van
Riel.
This patch contains the logic for validating several conditions when
performing copy_to_user() and copy_from_user() on the kernel object
being copied to/from:
- address range doesn't wrap around
- address range isn't NULL or zero-allocated (with a non-zero copy size)
- if on the slab allocator:
- object size must be less than or equal to copy size (when check is
implemented in the allocator, which appear in subsequent patches)
- otherwise, object must not span page allocations
- if on the stack
- object must not extend before/after the current process task
- object must be contained by the current stack frame (when there is
arch/build support for identifying stack frames)
- object must not overlap with kernel text
Signed-off-by: Kees Cook <redacted>
Tested-By: Valdis Kletnieks <redacted>
Tested-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/Kconfig | 7 ++
include/linux/slab.h | 12 +++
include/linux/thread_info.h | 15 +++
mm/Makefile | 4 +
mm/usercopy.c | 234
++++++++++++++++++++++++++++++++++++++++++++
security/Kconfig | 28 ++++++
6 files changed, 300 insertions(+)
create mode 100644 mm/usercopy.c
arch_within_stack_frames(),
which is used by CONFIG_HARDENED_USERCOPY.
+config HAVE_ARCH_LINEAR_KERNEL_MAPPING
+ bool
+ help
+ An architecture should select this if it has a secondary linear
+ mapping of the kernel text. This is used to verify that kernel
+ text exposures are not visible under CONFIG_HARDENED_USERCOPY.
+
config HAVE_CONTEXT_TRACKING
bool
help
@@ -21,6 +21,9 @@ KCOV_INSTRUMENT_memcontrol.o := nKCOV_INSTRUMENT_mmzone.o:=nKCOV_INSTRUMENT_vmstat.o:=n+# Since __builtin_frame_address does work as used, disable the warning.+CFLAGS_usercopy.o+=$(callcc-disable-warning,frame-address)+mmu-y:=nommu.ommu-$(CONFIG_MMU):=gup.ohighmem.omemory.omincore.o\mlock.ommap.omprotect.omremap.omsync.o
Nit: update comments to match enum (BAD_STACK instead of -1 etc.)
Ah, yes, thanks. I will fix this.
quoted
+static noinline int check_stack_object(const void *obj, unsigned long
len)
+{
+ const void * const stack = task_stack_page(current);
+ const void * const stackend = stack + THREAD_SIZE;
+ int ret;
+
+ /* Object is not on the stack at all. */
+ if (obj + len <= stack || stackend <= obj)
+ return NOT_STACK;
+
+ /*
+ * Reject: object partially overlaps the stack (passing the
+ * the check above means at least one end is within the stack,
+ * so if this check fails, the other end is outside the stack).
+ */
+ if (obj < stack || stackend < obj + len)
+ return BAD_STACK;
+
+ /* Check if object is safely within a valid frame. */
+ ret = arch_within_stack_frames(stack, stackend, obj, len);
+ if (ret)
+ return ret;
+
+ return GOOD_STACK;
+}
+
+static void report_usercopy(const void *ptr, unsigned long len,
+ bool to_user, const char *type)
+{
+ pr_emerg("kernel memory %s attempt detected %s %p (%s) (%lu
bytes)\n",
+ to_user ? "exposure" : "overwrite",
+ to_user ? "from" : "to", ptr, type ? : "unknown", len);
+ /*
+ * For greater effect, it would be nice to do do_group_exit(),
+ * but BUG() actually hooks all the lock-breaking and per-arch
+ * Oops code, so that is used here instead.
+ */
+ BUG();
+}
+
+/* Returns true if any portion of [ptr,ptr+n) over laps with [low,high).
*/
+static bool overlaps(const void *ptr, unsigned long n, unsigned long low,
+ unsigned long high)
+{
+ unsigned long check_low = (uintptr_t)ptr;
+ unsigned long check_high = check_low + n;
+
+ /* Does not overlap if entirely above or entirely below. */
+ if (check_low >= high || check_high < low)
+ return false;
+
+ return true;
+}
+
+/* Is this address range in the kernel text area? */
+static inline const char *check_kernel_text_object(const void *ptr,
+ unsigned long n)
+{
+ unsigned long textlow = (unsigned long)_stext;
+ unsigned long texthigh = (unsigned long)_etext;
+
+ if (overlaps(ptr, n, textlow, texthigh))
+ return "<kernel text>";
+
+#ifdef HAVE_ARCH_LINEAR_KERNEL_MAPPING
+ /* Check against linear mapping as well. */
+ if (overlaps(ptr, n, (unsigned long)__va(__pa(textlow)),
+ (unsigned long)__va(__pa(texthigh))))
+ return "<linear kernel text>";
+#endif
+
+ return NULL;
+}
+
+static inline const char *check_bogus_address(const void *ptr, unsigned
long n)
+{
+ /* Reject if object wraps past end of memory. */
+ if (ptr + n < ptr)
+ return "<wrapped address>";
+
+ /* Reject if NULL or ZERO-allocation. */
+ if (ZERO_OR_NULL_PTR(ptr))
+ return "<null>";
+
+ return NULL;
+}
+
+static inline const char *check_heap_object(const void *ptr, unsigned
long n,
+ bool to_user)
+{
+ struct page *page, *endpage;
+ const void *end = ptr + n - 1;
+
+ if (!virt_addr_valid(ptr))
+ return NULL;
+
+ page = virt_to_head_page(ptr);
+
+ /* Check slab allocator for flags and size. */
+ if (PageSlab(page))
+ return __check_heap_object(ptr, n, page);
+
+ /*
+ * Sometimes the kernel data regions are not marked Reserved (see
+ * check below). And sometimes [_sdata,_edata) does not cover
+ * rodata and/or bss, so check each range explicitly.
+ */
+
+ /* Allow reads of kernel rodata region (if not marked as
Reserved). */
+ if (ptr >= (const void *)__start_rodata &&
+ end <= (const void *)__end_rodata) {
+ if (!to_user)
+ return "<rodata>";
+ return NULL;
+ }
+
+ /* Allow kernel data region (if not marked as Reserved). */
+ if (ptr >= (const void *)_sdata && end <= (const void *)_edata)
+ return NULL;
+
+ /* Allow kernel bss region (if not marked as Reserved). */
+ if (ptr >= (const void *)__bss_start &&
+ end <= (const void *)__bss_stop)
+ return NULL;
+
+ /* Is the object wholly within one base page? */
+ if (likely(((unsigned long)ptr & (unsigned long)PAGE_MASK) ==
+ ((unsigned long)end & (unsigned long)PAGE_MASK)))
+ return NULL;
+
+ /* Allow if start and end are inside the same compound page. */
+ endpage = virt_to_head_page(end);
+ if (likely(endpage == page))
+ return NULL;
+
+ /*
+ * Reject if range is not Reserved (i.e. special or device
memory),
+ * since then the object spans several independently allocated
pages.
+ */
+ for (; ptr <= end ; ptr += PAGE_SIZE, page =
virt_to_head_page(ptr)) {
+ if (!PageReserved(page))
+ return "<spans multiple pages>";
+ }
+
This doesn't work when copying CMA allocated memory since CMA purposely
allocates larger than a page block size without setting head pages.
Given CMA may be used with drivers doing zero copy buffers, I think it
should be permitted.
Something like the following lets it pass (I can clean up and submit
the is_migrate_cma_page APIs as a separate patch for review)
Yeah, this would be great. I'd rather use an accessor to check this
than a direct check for MIGRATE_CMA.
*ptr, unsigned long n,
* since then the object spans several independently allocated
pages.
*/
for (; ptr <= end ; ptr += PAGE_SIZE, page = virt_to_head_page(ptr))
{
- if (!PageReserved(page))
+ if (!PageReserved(page) && !is_migrate_cma_page(page))
return "<spans multiple pages>";
}
Yeah, I'll modify this a bit so that which type it starts as is
maintained for all pages (rather than allowing to flip back and forth
-- even though that is likely impossible).
-Kees
--
Kees Cook
Chrome OS & Brillo Security
On Mon, Jul 18, 2016 at 6:52 PM, Laura Abbott [off-list ref] wrote:
On 07/15/2016 02:44 PM, Kees Cook wrote:
quoted
This is the start of porting PAX_USERCOPY into the mainline kernel. This
is the first set of features, controlled by CONFIG_HARDENED_USERCOPY. The
work is based on code by PaX Team and Brad Spengler, and an earlier port
from Casey Schaufler. Additional non-slab page tests are from Rik van
Riel.
This patch contains the logic for validating several conditions when
performing copy_to_user() and copy_from_user() on the kernel object
being copied to/from:
- address range doesn't wrap around
- address range isn't NULL or zero-allocated (with a non-zero copy size)
- if on the slab allocator:
- object size must be less than or equal to copy size (when check is
implemented in the allocator, which appear in subsequent patches)
- otherwise, object must not span page allocations
- if on the stack
- object must not extend before/after the current process task
- object must be contained by the current stack frame (when there is
arch/build support for identifying stack frames)
- object must not overlap with kernel text
Signed-off-by: Kees Cook <redacted>
Tested-By: Valdis Kletnieks <redacted>
Tested-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/Kconfig | 7 ++
include/linux/slab.h | 12 +++
include/linux/thread_info.h | 15 +++
mm/Makefile | 4 +
mm/usercopy.c | 234
++++++++++++++++++++++++++++++++++++++++++++
security/Kconfig | 28 ++++++
6 files changed, 300 insertions(+)
create mode 100644 mm/usercopy.c
arch_within_stack_frames(),
which is used by CONFIG_HARDENED_USERCOPY.
+config HAVE_ARCH_LINEAR_KERNEL_MAPPING
+ bool
+ help
+ An architecture should select this if it has a secondary linear
+ mapping of the kernel text. This is used to verify that kernel
+ text exposures are not visible under CONFIG_HARDENED_USERCOPY.
+
config HAVE_CONTEXT_TRACKING
bool
help
@@ -21,6 +21,9 @@ KCOV_INSTRUMENT_memcontrol.o := nKCOV_INSTRUMENT_mmzone.o:=nKCOV_INSTRUMENT_vmstat.o:=n+# Since __builtin_frame_address does work as used, disable the warning.+CFLAGS_usercopy.o+=$(callcc-disable-warning,frame-address)+mmu-y:=nommu.ommu-$(CONFIG_MMU):=gup.ohighmem.omemory.omincore.o\mlock.ommap.omprotect.omremap.omsync.o
len)
+{
+ const void * const stack = task_stack_page(current);
+ const void * const stackend = stack + THREAD_SIZE;
+ int ret;
+
+ /* Object is not on the stack at all. */
+ if (obj + len <= stack || stackend <= obj)
+ return NOT_STACK;
+
+ /*
+ * Reject: object partially overlaps the stack (passing the
+ * the check above means at least one end is within the stack,
+ * so if this check fails, the other end is outside the stack).
+ */
+ if (obj < stack || stackend < obj + len)
+ return BAD_STACK;
+
+ /* Check if object is safely within a valid frame. */
+ ret = arch_within_stack_frames(stack, stackend, obj, len);
+ if (ret)
+ return ret;
+
+ return GOOD_STACK;
+}
+
+static void report_usercopy(const void *ptr, unsigned long len,
+ bool to_user, const char *type)
+{
+ pr_emerg("kernel memory %s attempt detected %s %p (%s) (%lu
bytes)\n",
+ to_user ? "exposure" : "overwrite",
+ to_user ? "from" : "to", ptr, type ? : "unknown", len);
+ /*
+ * For greater effect, it would be nice to do do_group_exit(),
+ * but BUG() actually hooks all the lock-breaking and per-arch
+ * Oops code, so that is used here instead.
+ */
+ BUG();
+}
+
+/* Returns true if any portion of [ptr,ptr+n) over laps with [low,high).
*/
+static bool overlaps(const void *ptr, unsigned long n, unsigned long low,
+ unsigned long high)
+{
+ unsigned long check_low = (uintptr_t)ptr;
+ unsigned long check_high = check_low + n;
+
+ /* Does not overlap if entirely above or entirely below. */
+ if (check_low >= high || check_high < low)
+ return false;
+
+ return true;
+}
+
+/* Is this address range in the kernel text area? */
+static inline const char *check_kernel_text_object(const void *ptr,
+ unsigned long n)
+{
+ unsigned long textlow = (unsigned long)_stext;
+ unsigned long texthigh = (unsigned long)_etext;
+
+ if (overlaps(ptr, n, textlow, texthigh))
+ return "<kernel text>";
+
+#ifdef HAVE_ARCH_LINEAR_KERNEL_MAPPING
+ /* Check against linear mapping as well. */
+ if (overlaps(ptr, n, (unsigned long)__va(__pa(textlow)),
+ (unsigned long)__va(__pa(texthigh))))
+ return "<linear kernel text>";
+#endif
+
+ return NULL;
+}
+
+static inline const char *check_bogus_address(const void *ptr, unsigned
long n)
+{
+ /* Reject if object wraps past end of memory. */
+ if (ptr + n < ptr)
+ return "<wrapped address>";
+
+ /* Reject if NULL or ZERO-allocation. */
+ if (ZERO_OR_NULL_PTR(ptr))
+ return "<null>";
+
+ return NULL;
+}
+
+static inline const char *check_heap_object(const void *ptr, unsigned
long n,
+ bool to_user)
+{
+ struct page *page, *endpage;
+ const void *end = ptr + n - 1;
+
+ if (!virt_addr_valid(ptr))
+ return NULL;
+
virt_addr_valid returns true on vmalloc addresses on arm64 which causes some
intermittent false positives (tab completion in a qemu buildroot environment
was showing it fairly reliably). I think this is an arm64 bug because
virt_addr_valid should return true if and only if virt_to_page returns the
corresponding page. We can work around this for now by explicitly
checking against is_vmalloc_addr.
Hrm, that's weird. Sounds like a bug too, but I'll add a check for
is_vmalloc_addr() to catch it for now.
-Kees
Thanks,
Laura
quoted
+ page = virt_to_head_page(ptr);
+
+ /* Check slab allocator for flags and size. */
+ if (PageSlab(page))
+ return __check_heap_object(ptr, n, page);
+
+ /*
+ * Sometimes the kernel data regions are not marked Reserved (see
+ * check below). And sometimes [_sdata,_edata) does not cover
+ * rodata and/or bss, so check each range explicitly.
+ */
+
+ /* Allow reads of kernel rodata region (if not marked as
Reserved). */
+ if (ptr >= (const void *)__start_rodata &&
+ end <= (const void *)__end_rodata) {
+ if (!to_user)
+ return "<rodata>";
+ return NULL;
+ }
+
+ /* Allow kernel data region (if not marked as Reserved). */
+ if (ptr >= (const void *)_sdata && end <= (const void *)_edata)
+ return NULL;
+
+ /* Allow kernel bss region (if not marked as Reserved). */
+ if (ptr >= (const void *)__bss_start &&
+ end <= (const void *)__bss_stop)
+ return NULL;
+
+ /* Is the object wholly within one base page? */
+ if (likely(((unsigned long)ptr & (unsigned long)PAGE_MASK) ==
+ ((unsigned long)end & (unsigned long)PAGE_MASK)))
+ return NULL;
+
+ /* Allow if start and end are inside the same compound page. */
+ endpage = virt_to_head_page(end);
+ if (likely(endpage == page))
+ return NULL;
+
+ /*
+ * Reject if range is not Reserved (i.e. special or device
memory),
+ * since then the object spans several independently allocated
pages.
+ */
+ for (; ptr <= end ; ptr += PAGE_SIZE, page =
virt_to_head_page(ptr)) {
+ if (!PageReserved(page))
+ return "<spans multiple pages>";
+ }
+
+ return NULL;
+}
+
+/*
+ * Validates that the given object is one of:
+ * - known safe heap object
+ * - known safe stack object
+ * - not in kernel text
+ */
+void __check_object_size(const void *ptr, unsigned long n, bool to_user)
+{
+ const char *err;
+
+ /* Skip all tests if size is zero. */
+ if (!n)
+ return;
+
+ /* Check for invalid addresses. */
+ err = check_bogus_address(ptr, n);
+ if (err)
+ goto report;
+
+ /* Check for bad heap object. */
+ err = check_heap_object(ptr, n, to_user);
+ if (err)
+ goto report;
+
+ /* Check for bad stack object. */
+ switch (check_stack_object(ptr, n)) {
+ case NOT_STACK:
+ /* Object is not touching the current process stack. */
+ break;
+ case GOOD_FRAME:
+ case GOOD_STACK:
+ /*
+ * Object is either in the correct frame (when it
+ * is possible to check) or just generally on the
+ * process stack (when frame checking not available).
+ */
+ return;
+ default:
+ err = "<process stack>";
+ goto report;
+ }
+
+ /* Check for object in kernel to avoid text exposure. */
+ err = check_kernel_text_object(ptr, n);
+ if (!err)
+ return;
+
+report:
+ report_usercopy(ptr, n, to_user, err);
+}
+EXPORT_SYMBOL(__check_object_size);
On Tue, Jul 19, 2016 at 2:21 AM, Christian Borntraeger
[off-list ref] wrote:
On 07/15/2016 11:44 PM, Kees Cook wrote:
quoted
+config HAVE_ARCH_LINEAR_KERNEL_MAPPING
+ bool
+ help
+ An architecture should select this if it has a secondary linear
+ mapping of the kernel text. This is used to verify that kernel
+ text exposures are not visible under CONFIG_HARDENED_USERCOPY.
I have trouble parsing this. (What does secondary linear mapping mean?)
I likely need help clarifying this language...
So let me give an example below
quoted
+
[...]
quoted
+/* Is this address range in the kernel text area? */
+static inline const char *check_kernel_text_object(const void *ptr,
+ unsigned long n)
+{
+ unsigned long textlow = (unsigned long)_stext;
+ unsigned long texthigh = (unsigned long)_etext;
+
+ if (overlaps(ptr, n, textlow, texthigh))
+ return "<kernel text>";
+
+#ifdef HAVE_ARCH_LINEAR_KERNEL_MAPPING
+ /* Check against linear mapping as well. */
+ if (overlaps(ptr, n, (unsigned long)__va(__pa(textlow)),
+ (unsigned long)__va(__pa(texthigh))))
+ return "<linear kernel text>";
+#endif
+
+ return NULL;
+}
s390 has an address space for user (primary address space from 0..4TB/8PB) and a separate
address space (home space from 0..4TB/8PB) for the kernel. In this home space the kernel
mapping is virtual containing the physical memory as well as vmalloc memory (creating aliases
into the physical one). The kernel text is mapped from _stext to _etext in this mapping.
So I assume this would qualify for HAVE_ARCH_LINEAR_KERNEL_MAPPING ?
If I understand your example, yes. In the home space you have two
addresses that reference the kernel image? The intent is that if
__va(__pa(_stext)) != _stext, there's a linear mapping of physical
memory in the virtual memory range. On x86_64, the kernel is visible
in two locations in virtual memory. The kernel start in physical
memory address 0x01000000 maps to virtual address 0xffff880001000000,
and the "regular" virtual memory kernel address is at
0xffffffff81000000:
# grep Kernel /proc/iomem
01000000-01a59767 : Kernel code
01a59768-0213d77f : Kernel data
02280000-02fdefff : Kernel bss
# grep startup_64 /proc/kallsyms
ffffffff81000000 T startup_64
# less /sys/kernel/debug/kernel_page_tables
...
---[ Low Kernel Mapping ]---
...
0xffff880001000000-0xffff880001a00000 10M ro PSE
GLB NX pmd
0xffff880001a00000-0xffff880001a5c000 368K ro GLB NX pte
0xffff880001a5c000-0xffff880001c00000 1680K RW GLB NX pte
...
---[ High Kernel Mapping ]---
...
0xffffffff81000000-0xffffffff81a00000 10M ro PSE
GLB x pmd
0xffffffff81a00000-0xffffffff81a5c000 368K ro GLB x pte
0xffffffff81a5c000-0xffffffff81c00000 1680K RW GLB NX pte
...
I wonder if I can avoid the CONFIG entirely if I just did a
__va(__pa(_stext)) != _stext test... would that break anyone?
-Kees
--
Kees Cook
Chrome OS & Brillo Security
From: Christian Borntraeger <hidden> Date: 2016-07-19 20:14:42
On 07/19/2016 09:31 PM, Kees Cook wrote:
On Tue, Jul 19, 2016 at 2:21 AM, Christian Borntraeger
[off-list ref] wrote:
quoted
On 07/15/2016 11:44 PM, Kees Cook wrote:
quoted
+config HAVE_ARCH_LINEAR_KERNEL_MAPPING
+ bool
+ help
+ An architecture should select this if it has a secondary linear
+ mapping of the kernel text. This is used to verify that kernel
+ text exposures are not visible under CONFIG_HARDENED_USERCOPY.
I have trouble parsing this. (What does secondary linear mapping mean?)
I likely need help clarifying this language...
quoted
So let me give an example below
quoted
+
[...]
quoted
+/* Is this address range in the kernel text area? */
+static inline const char *check_kernel_text_object(const void *ptr,
+ unsigned long n)
+{
+ unsigned long textlow = (unsigned long)_stext;
+ unsigned long texthigh = (unsigned long)_etext;
+
+ if (overlaps(ptr, n, textlow, texthigh))
+ return "<kernel text>";
+
+#ifdef HAVE_ARCH_LINEAR_KERNEL_MAPPING
+ /* Check against linear mapping as well. */
+ if (overlaps(ptr, n, (unsigned long)__va(__pa(textlow)),
+ (unsigned long)__va(__pa(texthigh))))
+ return "<linear kernel text>";
+#endif
+
+ return NULL;
+}
s390 has an address space for user (primary address space from 0..4TB/8PB) and a separate
address space (home space from 0..4TB/8PB) for the kernel. In this home space the kernel
mapping is virtual containing the physical memory as well as vmalloc memory (creating aliases
into the physical one). The kernel text is mapped from _stext to _etext in this mapping.
So I assume this would qualify for HAVE_ARCH_LINEAR_KERNEL_MAPPING ?
If I understand your example, yes. In the home space you have two
addresses that reference the kernel image?
No, there is only one address that points to the kernel.
As we have no kernel ASLR yet, and the kernel mapping is
a 1:1 mapping from 0 to memory end and the kernel is only
from _stext to _etext. The vmalloc area contains modules
and vmalloc but not a 2nd kernel mapping.
But thanks for your example, now I understood. If we have only
one address
quoted
quoted
+ if (overlaps(ptr, n, textlow, texthigh))
+ return "<kernel text>";
This is just enough.
So what about for the CONFIG text:
An architecture should select this if the kernel mapping has a secondary
linear mapping of the kernel text - in other words more than one virtual
kernel address that points to the kernel image. This is used to verify
that kernel text exposures are not visible under CONFIG_HARDENED_USERCOPY.
I wonder if I can avoid the CONFIG entirely if I just did a
__va(__pa(_stext)) != _stext test... would that break anyone?
Can this be resolved on all platforms at compile time?
On Tue, Jul 19, 2016 at 1:14 PM, Christian Borntraeger
[off-list ref] wrote:
On 07/19/2016 09:31 PM, Kees Cook wrote:
quoted
On Tue, Jul 19, 2016 at 2:21 AM, Christian Borntraeger
[off-list ref] wrote:
quoted
On 07/15/2016 11:44 PM, Kees Cook wrote:
quoted
+config HAVE_ARCH_LINEAR_KERNEL_MAPPING
+ bool
+ help
+ An architecture should select this if it has a secondary linear
+ mapping of the kernel text. This is used to verify that kernel
+ text exposures are not visible under CONFIG_HARDENED_USERCOPY.
I have trouble parsing this. (What does secondary linear mapping mean?)
I likely need help clarifying this language...
quoted
So let me give an example below
quoted
+
[...]
quoted
+/* Is this address range in the kernel text area? */
+static inline const char *check_kernel_text_object(const void *ptr,
+ unsigned long n)
+{
+ unsigned long textlow = (unsigned long)_stext;
+ unsigned long texthigh = (unsigned long)_etext;
+
+ if (overlaps(ptr, n, textlow, texthigh))
+ return "<kernel text>";
+
+#ifdef HAVE_ARCH_LINEAR_KERNEL_MAPPING
+ /* Check against linear mapping as well. */
+ if (overlaps(ptr, n, (unsigned long)__va(__pa(textlow)),
+ (unsigned long)__va(__pa(texthigh))))
+ return "<linear kernel text>";
+#endif
+
+ return NULL;
+}
s390 has an address space for user (primary address space from 0..4TB/8PB) and a separate
address space (home space from 0..4TB/8PB) for the kernel. In this home space the kernel
mapping is virtual containing the physical memory as well as vmalloc memory (creating aliases
into the physical one). The kernel text is mapped from _stext to _etext in this mapping.
So I assume this would qualify for HAVE_ARCH_LINEAR_KERNEL_MAPPING ?
If I understand your example, yes. In the home space you have two
addresses that reference the kernel image?
No, there is only one address that points to the kernel.
As we have no kernel ASLR yet, and the kernel mapping is
a 1:1 mapping from 0 to memory end and the kernel is only
from _stext to _etext. The vmalloc area contains modules
and vmalloc but not a 2nd kernel mapping.
But thanks for your example, now I understood. If we have only
one address
quoted
quoted
quoted
+ if (overlaps(ptr, n, textlow, texthigh))
+ return "<kernel text>";
This is just enough.
So what about for the CONFIG text:
An architecture should select this if the kernel mapping has a secondary
linear mapping of the kernel text - in other words more than one virtual
kernel address that points to the kernel image. This is used to verify
that kernel text exposures are not visible under CONFIG_HARDENED_USERCOPY.
Sounds good, I've adjusted it for now.
quoted
I wonder if I can avoid the CONFIG entirely if I just did a
__va(__pa(_stext)) != _stext test... would that break anyone?
Can this be resolved on all platforms at compile time?
Well, I think it still needs a runtime check (compile-time may not be
able to tell about kaslr, or who knows what else). I would really like
to avoid the CONFIG if possible, though. Would this do the right thing
on s390? This appears to work where I'm able to test it (32/64 x86,
32/64 arm):
unsigned long textlow = (unsigned long)_stext;
unsigned long texthigh = (unsigned long)_etext;
unsigned long textlow_linear = (unsigned long)__va(__pa(textlow);
unsigned long texthigh_linear = (unsigned long)__va(__pa(texthigh);
if (overlaps(ptr, n, textlow, texthigh))
return "<kernel text>";
/* Check against possible secondary linear mapping as well. */
if (textlow != textlow_linear &&
overlaps(ptr, n, textlow_linear, texthigh_linear))
return "<linear kernel text>";
return NULL;
-Kees
--
Kees Cook
Chrome OS & Brillo Security
From: Christian Borntraeger <hidden> Date: 2016-07-19 20:45:14
On 07/19/2016 10:34 PM, Kees Cook wrote:
[...]
quoted
So what about for the CONFIG text:
An architecture should select this if the kernel mapping has a secondary
linear mapping of the kernel text - in other words more than one virtual
kernel address that points to the kernel image. This is used to verify
that kernel text exposures are not visible under CONFIG_HARDENED_USERCOPY.
Sounds good, I've adjusted it for now.
quoted
quoted
I wonder if I can avoid the CONFIG entirely if I just did a
__va(__pa(_stext)) != _stext test... would that break anyone?
Can this be resolved on all platforms at compile time?
Well, I think it still needs a runtime check (compile-time may not be
able to tell about kaslr, or who knows what else). I would really like
to avoid the CONFIG if possible, though. Would this do the right thing
on s390? This appears to work where I'm able to test it (32/64 x86,
32/64 arm):
unsigned long textlow = (unsigned long)_stext;
unsigned long texthigh = (unsigned long)_etext;
unsigned long textlow_linear = (unsigned long)__va(__pa(textlow);
unsigned long texthigh_linear = (unsigned long)__va(__pa(texthigh);
as we have
#define PAGE_OFFSET 0x0UL
#define __pa(x) (unsigned long)(x)
#define __va(x) (void *)(unsigned long)(x)
both should be identical on s390 as of today, so it should work fine and only
do the check once
if (overlaps(ptr, n, textlow, texthigh))
return "<kernel text>";
/* Check against possible secondary linear mapping as well. */
if (textlow != textlow_linear &&
overlaps(ptr, n, textlow_linear, texthigh_linear))
return "<linear kernel text>";
return NULL;
-Kees
PS: Not sure how useful and flexible this offers is but you can get some temporary
free access to an s390 on https://developer.ibm.com/linuxone/
From: Laura Abbott <hidden> Date: 2016-07-19 22:00:19
Code such as hardened user copy[1] needs a way to tell if a
page is CMA or not. Add is_migrate_cma_page in a similar way
to is_migrate_isolate_page.
[1]http://article.gmane.org/gmane.linux.kernel.mm/155238
Signed-off-by: Laura Abbott <redacted>
---
Here's an explicit patch, slightly different than what I posted before. It can
be kept separate or folded in as needed.
---
include/linux/mmzone.h | 2 ++
1 file changed, 2 insertions(+)
On Tue, Jul 19, 2016 at 3:00 PM, Laura Abbott [off-list ref] wrote:
Code such as hardened user copy[1] needs a way to tell if a
page is CMA or not. Add is_migrate_cma_page in a similar way
to is_migrate_isolate_page.
[1]http://article.gmane.org/gmane.linux.kernel.mm/155238
Signed-off-by: Laura Abbott <redacted>
Great, thanks!
---
Here's an explicit patch, slightly different than what I posted before. It can
be kept separate or folded in as needed.
Assuming there's no objection, I'll add it to my tree and use the new macro.
-Kees
On Tue, Jul 19, 2016 at 12:12 PM, Kees Cook [off-list ref] wrote:
On Mon, Jul 18, 2016 at 6:52 PM, Laura Abbott [off-list ref] wrote:
quoted
On 07/15/2016 02:44 PM, Kees Cook wrote:
quoted
+static inline const char *check_heap_object(const void *ptr, unsigned
long n,
+ bool to_user)
+{
+ struct page *page, *endpage;
+ const void *end = ptr + n - 1;
+
+ if (!virt_addr_valid(ptr))
+ return NULL;
+
virt_addr_valid returns true on vmalloc addresses on arm64 which causes some
intermittent false positives (tab completion in a qemu buildroot environment
was showing it fairly reliably). I think this is an arm64 bug because
virt_addr_valid should return true if and only if virt_to_page returns the
corresponding page. We can work around this for now by explicitly
checking against is_vmalloc_addr.
Hrm, that's weird. Sounds like a bug too, but I'll add a check for
is_vmalloc_addr() to catch it for now.
BTW, if you were testing against -next, KASAN moved things around in
copy_*_user() in a way I wasn't expecting (__copy* and copy* now both
call __arch_copy* instead of copy* calling __copy*). I'll have this
fixed in the next version.
-Kees
--
Kees Cook
Chrome OS & Brillo Security
On Tue, 2016-07-19 at 11:48 -0700, Kees Cook wrote:
On Mon, Jul 18, 2016 at 6:06 PM, Laura Abbott [off-list ref] wrote:
quoted
On 07/15/2016 02:44 PM, Kees Cook wrote:
This doesn't work when copying CMA allocated memory since CMA purposely
allocates larger than a page block size without setting head pages.
Given CMA may be used with drivers doing zero copy buffers, I think it
should be permitted.
Something like the following lets it pass (I can clean up and submit
the is_migrate_cma_page APIs as a separate patch for review)
Yeah, this would be great. I'd rather use an accessor to check this
than a direct check for MIGRATE_CMA.
quoted
*/
for (; ptr <= end ; ptr += PAGE_SIZE, page = virt_to_head_page(ptr))
{
- if (!PageReserved(page))
+ if (!PageReserved(page) && !is_migrate_cma_page(page))
return "<spans multiple pages>";
}
Yeah, I'll modify this a bit so that which type it starts as is
maintained for all pages (rather than allowing to flip back and forth
-- even though that is likely impossible).
Sorry, I completely missed the MIGRATE_CMA bits. Could you clarify if you
caught this in testing/review?
Balbir Singh.
On Wed, Jul 20, 2016 at 2:52 AM, David Laight [off-list ref] wrote:
From: Kees Cook
quoted
Sent: 15 July 2016 22:44
This is a start of the mainline port of PAX_USERCOPY[1].
...
quoted
- if address range is in the current process stack, it must be within the
current stack frame (if such checking is possible) or at least entirely
within the current process's stack.
...
That description doesn't seem quite right to me.
I presume the check is:
Within the current process's stack and not crossing the ends of the
current stack frame.
Actually, it's a bad description all around. :) The check is that the
range is within a valid stack frame (current or any prior caller's
frame). i.e. it does not cross a frame or touch the saved frame
pointer nor instruction pointer.
The 'current' stack frame is likely to be that of copy_to/from_user().
Even if you use the stack of the caller, any problematic buffers
are likely to have been passed in from a calling function.
So unless you are going to walk the stack (good luck on that)
I'm not sure checking the stack frames is worth it.
Yup: that's exactly what it's doing: walking up the stack. :)
-Kees
--
Kees Cook
Chrome OS & Brillo Security
From: Laura Abbott <hidden> Date: 2016-07-20 15:36:52
On 07/20/2016 03:24 AM, Balbir Singh wrote:
On Tue, 2016-07-19 at 11:48 -0700, Kees Cook wrote:
quoted
On Mon, Jul 18, 2016 at 6:06 PM, Laura Abbott [off-list ref] wrote:
quoted
On 07/15/2016 02:44 PM, Kees Cook wrote:
This doesn't work when copying CMA allocated memory since CMA purposely
allocates larger than a page block size without setting head pages.
Given CMA may be used with drivers doing zero copy buffers, I think it
should be permitted.
Something like the following lets it pass (I can clean up and submit
the is_migrate_cma_page APIs as a separate patch for review)
Yeah, this would be great. I'd rather use an accessor to check this
than a direct check for MIGRATE_CMA.
quoted
*/
for (; ptr <= end ; ptr += PAGE_SIZE, page = virt_to_head_page(ptr))
{
- if (!PageReserved(page))
+ if (!PageReserved(page) && !is_migrate_cma_page(page))
return "<spans multiple pages>";
}
Yeah, I'll modify this a bit so that which type it starts as is
maintained for all pages (rather than allowing to flip back and forth
-- even though that is likely impossible).
Sorry, I completely missed the MIGRATE_CMA bits. Could you clarify if you
caught this in testing/review?
Balbir Singh.
I caught it while looking at the code and then wrote a test case to confirm
I was correct because I wasn't sure how to easily find an in tree user.
Thanks,
Laura
+
+/*
+ * Checks if a given pointer and length is contained by the current
+ * stack frame (if possible).
+ *
+ * 0: not at all on the stack
+ * 1: fully within a valid stack frame
+ * 2: fully on the stack (when can't do frame-checking)
+ * -1: error condition (invalid stack position or bad stack frame)
+ */
+static noinline int check_stack_object(const void *obj, unsigned long len)
+{
+ const void * const stack = task_stack_page(current);
+ const void * const stackend = stack + THREAD_SIZE;
That allows access to the entire stack, including the struct thread_info,
is that what we want - it seems dangerous? Or did I miss a check
somewhere else?
We have end_of_stack() which computes the end of the stack taking
thread_info into account (end being the opposite of your end above).
cheers