[PATCH] include: mman: Use bool instead of int for the return value of arch_validate_prot

[chengang]

From: Chen Gang 

For pure bool function's return value, bool is a little better more or
less than int.

And return boolean result directly. Since 'if' statement is also for
boolean checking, and return boolean result, too.

Signed-off-by: Chen Gang 
---
 arch/powerpc/include/asm/mman.h | 8 +++-
 include/linux/mman.h| 2 +-
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/include/asm/mman.h b/arch/powerpc/include/asm/mman.h
index 2563c43..62e1f47 100644
--- a/arch/powerpc/include/asm/mman.h
+++ b/arch/powerpc/include/asm/mman.h
@@ -31,13 +31,11 @@ static inline pgprot_t arch_vm_get_page_prot(unsigned long 
vm_flags)
 }
 #define arch_vm_get_page_prot(vm_flags) arch_vm_get_page_prot(vm_flags)
 
-static inline int arch_validate_prot(unsigned long prot)
+static inline bool arch_validate_prot(unsigned long prot)
 {
if (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM | PROT_SAO))
-   return 0;
-   if ((prot & PROT_SAO) && !cpu_has_feature(CPU_FTR_SAO))
-   return 0;
-   return 1;
+   return false;
+   return (prot & PROT_SAO) == 0 || cpu_has_feature(CPU_FTR_SAO);
 }
 #define arch_validate_prot(prot) arch_validate_prot(prot)
 
diff --git a/include/linux/mman.h b/include/linux/mman.h
index 33e17f6..634c4c5 100644
--- a/include/linux/mman.h
+++ b/include/linux/mman.h
@@ -49,7 +49,7 @@ static inline void vm_unacct_memory(long pages)
  *
  * Returns true if the prot flags are valid
  */
-static inline int arch_validate_prot(unsigned long prot)
+static inline bool arch_validate_prot(unsigned long prot)
 {
return (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM)) == 0;
 }
-- 
1.9.3

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 0/9] mm: Hardened usercopy

[PaX Team]

On 9 Jul 2016 at 14:27, Andy Lutomirski wrote:

> On Jul 6, 2016 6:25 PM, "Kees Cook"  wrote:
> >
> > Hi,
> >
> > 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's feedback and made a number
> > of other changes and clean-ups as well.
> >
> 
> I like the series, but I have one minor nit to pick.  The effect of
> this series is to harden usercopy, but most of the code is really
> about infrastructure to validate that a pointed-to object is valid.

actually USERCOPY has never been about validating pointers. its sole purpose
is to validate the *size* argument of copy*user calls, a very specific form
of runtime bounds checking. it's only really relevant for slab objects and the
pointer checks (that one might mistake for being a part of the defense 
mechanism)
are only there to determine whether the kernel pointer refers to a slab object
or not (the stack part is a small bonus and was never the main goal either).

> Might it make sense to call the infrastructure part something else?

yes, more bikeshedding will surely help, like the renaming of .data..read_only
to .data..ro_after_init which also had nothing to do with init but everything
to do with objects being conceptually read-only...

> After all, this could be extended in the future for memcpy or even for
> some GCC plugin to check pointers passed to ordinary (non-allocator)
> functions.

what kind of checks are you thinking of here? and more fundamentally, against
what kind of threats? as for memcpy, it's the standard mandated memory copying
function, what security related properties can it check on its pointer 
arguments?

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] Need proper type casting before assignment, Remove compilation Warning.

[David Miller]

From: Arvind Yadav 
Date: Fri,  8 Jul 2016 00:07:54 +0530

> -Return type of 'qe_muram_alloc' is 'unsigned long', That Was trying to
> assigned in ucc_fast_tx_virtual_fifo_base_offset and
> ucc_fast_rx_virtual_fifo_base_offset. These variable are 'unsigned int'.
> So before assginment need a proper type casting.
> 
> -Passing value in IS_ERR_VALUE() is wrong, as they pass an 'int'
> into a function that takes an 'unsigned long' argument.This happens
> to work because the type is sign-extended on 64-bit architectures
> before it gets converted into an unsigned type.
> 
> -Passing an 'unsigned short' or 'unsigned int'argument into
> IS_ERR_VALUE() is guaranteed to be broken, as are 8-bit integers
> and types that are wider than 'unsigned long'.
> 
> -Any user will get compilation warning for that do not pass an
> unsigned long' argument.
> 
> Signed-off-by: Arvind Yadav 

Your subject line is improperly formed.

It must have the subsystem or driver name, followed by a colon ":"
and a space.  Such as:

[PATCH] ucc_geth: Need proper type ...

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 0/9] mm: Hardened usercopy

[Andy Lutomirski]

On Jul 6, 2016 6:25 PM, "Kees Cook"  wrote:
>
> Hi,
>
> 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's feedback and made a number
> of other changes and clean-ups as well.
>

I like the series, but I have one minor nit to pick.  The effect of
this series is to harden usercopy, but most of the code is really
about infrastructure to validate that a pointed-to object is valid.
Might it make sense to call the infrastructure part something else?
After all, this could be extended in the future for memcpy or even for
some GCC plugin to check pointers passed to ordinary (non-allocator)
functions.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [kernel-hardening] Re: [PATCH 9/9] mm: SLUB hardened usercopy support

[Kees Cook]

On Fri, Jul 8, 2016 at 11:17 PM,   wrote:
> Yeah, 'ping' dies with a similar traceback going to rawv6_setsockopt(),
> and 'trinity' dies a horrid death during initialization because it creates
> some sctp sockets to fool around with.  The problem in all these cases is that
> setsockopt uses copy_from_user() to pull in the option value, and the 
> allocation
> isn't tagged with USERCOPY to whitelist it.

Just a note to clear up confusion: this series doesn't include the
whitelist protection, so this appears to be either bugs in the slub
checker or bugs in the code using the cfq_io_cq cache. I suspect the
former. :)

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 0/9] mm: Hardened usercopy

[Kees Cook]

On Sat, Jul 9, 2016 at 1:25 AM, Ard Biesheuvel
 wrote:
> On 9 July 2016 at 04:22, Laura Abbott  wrote:
>> On 07/06/2016 03:25 PM, Kees Cook wrote:
>>>
>>> Hi,
>>>
>>> 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's feedback and made a number
>>> of other changes and clean-ups as well.
>>>
>>> Based on my understanding, PAX_USERCOPY was designed to catch a few
>>> classes of flaws 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:
>>> - The core copy_to/from_user() checks, without the slab object checks:
>>> 1- mm: Hardened usercopy
>>> - Per-arch enablement of the protection:
>>> 2- x86/uaccess: Enable hardened usercopy
>>> 3- ARM: uaccess: Enable hardened usercopy
>>> 4- arm64/uaccess: Enable hardened usercopy
>>> 5- ia64/uaccess: Enable hardened usercopy
>>> 6- powerpc/uaccess: Enable hardened usercopy
>>> 7- sparc/uaccess: Enable hardened usercopy
>>> - The heap allocator implementation of object size checking:
>>> 8- mm: SLAB hardened usercopy support
>>> 9- 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.
>>>
>>> - 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.
>>>
>>> - 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).
>>>
>>>
>>
>> Even with the SLUB fixup I'm still seeing this blow up on my arm64 system.
>> This is a
>> Fedora rawhide kernel + the patches
>>
>> [ 0.666700] usercopy: kernel memory exposure attempt detected from
>> fc0008b4dd58 () (8 bytes)
>> [ 0.666720] CPU: 2 PID: 79 Comm: modprobe Tainted: GW
>> 4.7.0-0.rc6.git1.1.hardenedusercopy.fc25.aarch64 #1
>> [ 0.666733] Hardware name: AppliedMicro Mustang/Mustang, BIOS 1.1.0 Nov 24
>> 2015
>> [ 0.666744] Call trace:
>> [ 0.666756] [] dump_backtrace+0x0/0x1e8
>> [ 0.666765] [] show_stack+0x24/0x30
>> [ 0.666775] [] dump_stack+0xa4/0xe0
>> [ 0.666785] [] __check_object_size+0x6c/0x230
>> [ 0.666795] [] create_elf_tables+0x74/0x420
>> [ 0.666805] [] load_elf_binary+0x828/0xb70
>> [ 0.666814] [] search_binary_handler+0xb4/0x240
>> [ 0.666823] [] do_execveat_common+0x63c/0x950
>> [ 0.666832] [] do_execve+0x3c/0x50
>> [ 0.666841] [] call_usermodehelper_exec_async+0xe8/0x148
>> [ 0.666850] [] ret_from_fork+0x10/0x50
>>
>> This happens on every call to execve. This seems to be the first
>> copy_to_user in
>> create_elf_tables. I didn't get a chance to debug and I'm going out of town
>> all of next week so all I have is the report unfortunately. config attached.
>>
>
> This is a known issue, and a fix is already queued for v4.8 in the arm64 tree:
>
> 

Re: [PATCH 0/9] mm: Hardened usercopy

[Kees Cook]

On Fri, Jul 8, 2016 at 7:22 PM, Laura Abbott  wrote:
> On 07/06/2016 03:25 PM, Kees Cook wrote:
>>
>> Hi,
>>
>> 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's feedback and made a number
>> of other changes and clean-ups as well.
>>
>> Based on my understanding, PAX_USERCOPY was designed to catch a few
>> classes of flaws 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:
>> - The core copy_to/from_user() checks, without the slab object checks:
>> 1- mm: Hardened usercopy
>> - Per-arch enablement of the protection:
>> 2- x86/uaccess: Enable hardened usercopy
>> 3- ARM: uaccess: Enable hardened usercopy
>> 4- arm64/uaccess: Enable hardened usercopy
>> 5- ia64/uaccess: Enable hardened usercopy
>> 6- powerpc/uaccess: Enable hardened usercopy
>> 7- sparc/uaccess: Enable hardened usercopy
>> - The heap allocator implementation of object size checking:
>> 8- mm: SLAB hardened usercopy support
>> 9- 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.
>>
>> - 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.
>>
>> - 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).
>>
>>
>
> Even with the SLUB fixup I'm still seeing this blow up on my arm64 system.
> This is a
> Fedora rawhide kernel + the patches

Is this on top of -next? The recent _etext change ("arm64: mm: fix
location of _etext") is needed to fix the kernel text test for arm64.

-Kees

>
> [0.666700] usercopy: kernel memory exposure attempt detected from
> fc0008b4dd58 () (8 bytes)
> [0.666720] CPU: 2 PID: 79 Comm: modprobe Tainted: GW
> 4.7.0-0.rc6.git1.1.hardenedusercopy.fc25.aarch64 #1
> [0.666733] Hardware name: AppliedMicro Mustang/Mustang, BIOS 1.1.0 Nov
> 24 2015
> [0.666744] Call trace:
> [0.666756] [] dump_backtrace+0x0/0x1e8
> [0.666765] [] show_stack+0x24/0x30
> [0.666775] [] dump_stack+0xa4/0xe0
> [0.666785] [] __check_object_size+0x6c/0x230
> [0.666795] [] create_elf_tables+0x74/0x420
> [0.666805] [] load_elf_binary+0x828/0xb70
> [0.666814] [] search_binary_handler+0xb4/0x240
> [0.666823] [] do_execveat_common+0x63c/0x950
> [0.666832] [] do_execve+0x3c/0x50
> [0.666841] []
> call_usermodehelper_exec_async+0xe8/0x148
> [0.666850] [] ret_from_fork+0x10/0x50
>
> This happens on every call to execve. This seems to be the first
> copy_to_user in
> create_elf_tables. I didn't get a chance to debug and I'm going out of town
> all of next week so all I have is the report unfortunately. config attached.
>
> Thanks,
> Laura



-- 
Kees Cook
Chrome OS & Brillo Security

Re: [PATCH 0/9] mm: Hardened usercopy

[Laura Abbott]

On Sat, Jul 9, 2016 at 1:25 AM, Ard Biesheuvel 
wrote:

> On 9 July 2016 at 04:22, Laura Abbott  wrote:
> > On 07/06/2016 03:25 PM, Kees Cook wrote:
> >>
> >> Hi,
> >>
> >> 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's feedback and made a number
> >> of other changes and clean-ups as well.
> >>
> >> Based on my understanding, PAX_USERCOPY was designed to catch a few
> >> classes of flaws 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:
> >> - The core copy_to/from_user() checks, without the slab object checks:
> >> 1- mm: Hardened usercopy
> >> - Per-arch enablement of the protection:
> >> 2- x86/uaccess: Enable hardened usercopy
> >> 3- ARM: uaccess: Enable hardened usercopy
> >> 4- arm64/uaccess: Enable hardened usercopy
> >> 5- ia64/uaccess: Enable hardened usercopy
> >> 6- powerpc/uaccess: Enable hardened usercopy
> >> 7- sparc/uaccess: Enable hardened usercopy
> >> - The heap allocator implementation of object size checking:
> >> 8- mm: SLAB hardened usercopy support
> >> 9- 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.
> >>
> >> - 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.
> >>
> >> - 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).
> >>
> >>
> >
> > Even with the SLUB fixup I'm still seeing this blow up on my arm64
> system.
> > This is a
> > Fedora rawhide kernel + the patches
> >
> > [ 0.666700] usercopy: kernel memory exposure attempt detected from
> > fc0008b4dd58 () (8 bytes)
> > [ 0.666720] CPU: 2 PID: 79 Comm: modprobe Tainted: GW
> > 4.7.0-0.rc6.git1.1.hardenedusercopy.fc25.aarch64 #1
> > [ 0.666733] Hardware name: AppliedMicro Mustang/Mustang, BIOS 1.1.0 Nov
> 24
> > 2015
> > [ 0.666744] Call trace:
> > [ 0.666756] [] dump_backtrace+0x0/0x1e8
> > [ 0.666765] [] show_stack+0x24/0x30
> > [ 0.666775] [] dump_stack+0xa4/0xe0
> > [ 0.666785] [] __check_object_size+0x6c/0x230
> > [ 0.666795] [] create_elf_tables+0x74/0x420
> > [ 0.666805] [] load_elf_binary+0x828/0xb70
> > [ 0.666814] [] search_binary_handler+0xb4/0x240
> > [ 0.666823] [] do_execveat_common+0x63c/0x950
> > [ 0.666832] [] do_execve+0x3c/0x50
> > [ 0.666841] []
> call_usermodehelper_exec_async+0xe8/0x148
> > [ 0.666850] [] ret_from_fork+0x10/0x50
> >
> > This happens on every call to execve. This seems to be the first
> > copy_to_user in
> > create_elf_tables. I didn't get a chance to debug and I'm going out of
> town
> > all of next week so all I have is the report 

[PATCH] powerpc/8xx: Force VIRT_IMMR_BASE to be a positive number

[Scott Wood]

The asm-offsets mechanism generates signed numbers, even if the
input value is explicitly unsigned.  This causes a problem with
older binutils (e.g. 2.23), which sign-extend a negative number
when @h is applied.  Thus, this instruction:

cmpli   cr0, r11, VIRT_IMMR_BASE@h

resulted in this:

Error: operand out of range (0xfff0 is not between 0x and
0x)

By casting to a larger type, we can force the output to be expressed
as a positive number.

Signed-off-by: Scott Wood 
Cc: Christophe Leroy 
---
 arch/powerpc/kernel/asm-offsets.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/asm-offsets.c 
b/arch/powerpc/kernel/asm-offsets.c
index 247f640..b89d14c 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -754,7 +754,7 @@ int main(void)
DEFINE(PPC_DBELL_SERVER, PPC_DBELL_SERVER);
 
 #ifdef CONFIG_PPC_8xx
-   DEFINE(VIRT_IMMR_BASE, __fix_to_virt(FIX_IMMR_BASE));
+   DEFINE(VIRT_IMMR_BASE, (u64)__fix_to_virt(FIX_IMMR_BASE));
 #endif
 
return 0;
-- 
2.7.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 0/9] mm: Hardened usercopy

[Ard Biesheuvel]

On 9 July 2016 at 04:22, Laura Abbott  wrote:
> On 07/06/2016 03:25 PM, Kees Cook wrote:
>>
>> Hi,
>>
>> 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's feedback and made a number
>> of other changes and clean-ups as well.
>>
>> Based on my understanding, PAX_USERCOPY was designed to catch a few
>> classes of flaws 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:
>> - The core copy_to/from_user() checks, without the slab object checks:
>> 1- mm: Hardened usercopy
>> - Per-arch enablement of the protection:
>> 2- x86/uaccess: Enable hardened usercopy
>> 3- ARM: uaccess: Enable hardened usercopy
>> 4- arm64/uaccess: Enable hardened usercopy
>> 5- ia64/uaccess: Enable hardened usercopy
>> 6- powerpc/uaccess: Enable hardened usercopy
>> 7- sparc/uaccess: Enable hardened usercopy
>> - The heap allocator implementation of object size checking:
>> 8- mm: SLAB hardened usercopy support
>> 9- 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.
>>
>> - 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.
>>
>> - 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).
>>
>>
>
> Even with the SLUB fixup I'm still seeing this blow up on my arm64 system.
> This is a
> Fedora rawhide kernel + the patches
>
> [ 0.666700] usercopy: kernel memory exposure attempt detected from
> fc0008b4dd58 () (8 bytes)
> [ 0.666720] CPU: 2 PID: 79 Comm: modprobe Tainted: GW
> 4.7.0-0.rc6.git1.1.hardenedusercopy.fc25.aarch64 #1
> [ 0.666733] Hardware name: AppliedMicro Mustang/Mustang, BIOS 1.1.0 Nov 24
> 2015
> [ 0.666744] Call trace:
> [ 0.666756] [] dump_backtrace+0x0/0x1e8
> [ 0.666765] [] show_stack+0x24/0x30
> [ 0.666775] [] dump_stack+0xa4/0xe0
> [ 0.666785] [] __check_object_size+0x6c/0x230
> [ 0.666795] [] create_elf_tables+0x74/0x420
> [ 0.666805] [] load_elf_binary+0x828/0xb70
> [ 0.666814] [] search_binary_handler+0xb4/0x240
> [ 0.666823] [] do_execveat_common+0x63c/0x950
> [ 0.666832] [] do_execve+0x3c/0x50
> [ 0.666841] [] call_usermodehelper_exec_async+0xe8/0x148
> [ 0.666850] [] ret_from_fork+0x10/0x50
>
> This happens on every call to execve. This seems to be the first
> copy_to_user in
> create_elf_tables. I didn't get a chance to debug and I'm going out of town
> all of next week so all I have is the report unfortunately. config attached.
>

This is a known issue, and a fix is already queued for v4.8 in the arm64 tree:

9fdc14c55c arm64: mm: fix location of _etext [0]

which moves _etext up in the linker script so that it does not cover .rodata

ARM was suffering from the same problem, and Kees proposed a 

Re: [PATCH 0/9] mm: Hardened usercopy

[Ingo Molnar]

* Rik van Riel  wrote:

> On Fri, 2016-07-08 at 19:22 -0700, Laura Abbott wrote:
> > 
> > Even with the SLUB fixup I'm still seeing this blow up on my arm64
> > system. This is a
> > Fedora rawhide kernel + the patches
> > 
> > [0.666700] usercopy: kernel memory exposure attempt detected from
> > fc0008b4dd58 () (8 bytes)
> > [0.666720] CPU: 2 PID: 79 Comm: modprobe Tainted:
> > GW   4.7.0-0.rc6.git1.1.hardenedusercopy.fc25.aarch64 #1
> > [0.666733] Hardware name: AppliedMicro Mustang/Mustang, BIOS
> > 1.1.0 Nov 24 2015
> > [0.666744] Call trace:
> > [0.666756] [] dump_backtrace+0x0/0x1e8
> > [0.666765] [] show_stack+0x24/0x30
> > [0.666775] [] dump_stack+0xa4/0xe0
> > [0.666785] [] __check_object_size+0x6c/0x230
> > [0.666795] [] create_elf_tables+0x74/0x420
> > [0.666805] [] load_elf_binary+0x828/0xb70
> > [0.666814] [] search_binary_handler+0xb4/0x240
> > [0.666823] [] do_execveat_common+0x63c/0x950
> > [0.666832] [] do_execve+0x3c/0x50
> > [0.666841] []
> > call_usermodehelper_exec_async+0xe8/0x148
> > [0.666850] [] ret_from_fork+0x10/0x50
> > 
> > This happens on every call to execve. This seems to be the first
> > copy_to_user in
> > create_elf_tables. I didn't get a chance to debug and I'm going out
> > of town
> > all of next week so all I have is the report unfortunately. config
> > attached.
> 
> That's odd, this should be copying a piece of kernel data (not text)
> to userspace.
> 
> from fs/binfmt_elf.c
> 
>         const char *k_platform = ELF_PLATFORM;
> 
> ...
>                 size_t len = strlen(k_platform) + 1;
>   
>                 u_platform = (elf_addr_t __user *)STACK_ALLOC(p, len);
> if (__copy_to_user(u_platform, k_platform, len))
> return -EFAULT;
> 
> from arch/arm/include/asm/elf.h:
> 
> #define ELF_PLATFORM_SIZE 8
> #define ELF_PLATFORM(elf_platform)
> 
> extern char elf_platform[];
> 
> from arch/arm/kernel/setup.c:
> 
> char elf_platform[ELF_PLATFORM_SIZE];
> EXPORT_SYMBOL(elf_platform);
> 
> ...
> 
> snprintf(elf_platform, ELF_PLATFORM_SIZE, "%s%c",
>  list->elf_name, ENDIANNESS);
> 
> How does that end up in the .text section of the
> image, instead of in one of the various data sections?
> 
> What kind of linker oddity is going on with ARM?

I think the crash happened on ARM64, not ARM.

Thanks,

Ingo
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v3 2/2] powerpc32: fix check_io_access()

[Scott Wood]

On Tue, 2016-05-17 at 14:01 +0200, Christophe Leroy wrote:
> On processors like the 8xx, the machine check exception can also
> happen directly on the load/store instruction itself, so that case
> needs to be handled as well
> 
> Signed-off-by: Christophe Leroy 
> ---

What machine checks are happening that this is handling?  Is there still 8xx
code that probes for registers that might not be there?

-Scott

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [kernel-hardening] Re: [PATCH 9/9] mm: SLUB hardened usercopy support

[Valdis . Kletnieks]

On Sat, 09 Jul 2016 15:58:20 +1000, Michael Ellerman said:

> I then get two hits, which may or may not be valid:
>
> [2.309556] usercopy: kernel memory overwrite attempt detected to 
> d3510028 (kernfs_node_cache) (64 bytes)
> [2.309995] CPU: 7 PID: 2241 Comm: wait-for-root Not tainted 
> 4.7.0-rc3-00099-g97872fc89d41 #64
> [2.310480] Call Trace:
> [2.310556] [c001f4773bf0] [c09bdbe8] dump_stack+0xb0/0xf0 
> (unreliable)
> [2.311016] [c001f4773c30] [c029cf44] 
> __check_object_size+0x74/0x320
> [2.311472] [c001f4773cb0] [c005d4d0] copy_from_user+0x60/0xd4
> [2.311873] [c001f4773cf0] [c08b38f4] __get_filter+0x74/0x160
> [2.312230] [c001f4773d30] [c08b408c] 
> sk_attach_filter+0x2c/0xc0
> [2.312596] [c001f4773d60] [c0871c34] 
> sock_setsockopt+0x954/0xc00
> [2.313021] [c001f4773dd0] [c086ac44] 
> SyS_setsockopt+0x134/0x150
> [2.313380] [c001f4773e30] [c0009260] system_call+0x38/0x108

Yeah, 'ping' dies with a similar traceback going to rawv6_setsockopt(),
and 'trinity' dies a horrid death during initialization because it creates
some sctp sockets to fool around with.  The problem in all these cases is that
setsockopt uses copy_from_user() to pull in the option value, and the allocation
isn't tagged with USERCOPY to whitelist it.

Unfortunately, I haven't been able to track down where in net/ the memory is
allocated, nor is there any good hint in the grsecurity patch that I can find
where they do the tagging.

And the fact that so far, I'm only had ping and trinity killed in setsockopt()
hints that *most* setsockopt() calls must be going through a code path that
does allocate suitable memory, and these two have different paths.  I can't
believe they're the only two binaries that call setsockopt().

Just saw your second mail, now I'm wondering why *my* laptop doesn't die a
horrid death when systemd starts up.  Mine is
systemd-230-3.gitea68351.fc25.x86_64 - maybe there's something
release-dependent going on?



pgpYxw2xx1BKc.pgp
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [kernel-hardening] Re: [PATCH 9/9] mm: SLUB hardened usercopy support

[Michael Ellerman]

Michael Ellerman  writes:

> Kees Cook  writes:
>
>> On Fri, Jul 8, 2016 at 1:41 PM, Kees Cook  wrote:
>>> So, as found already, the position in the usercopy check needs to be
>>> bumped down by red_left_pad, which is what Michael's fix does, so I'll
>>> include it in the next version.
>>
>> Actually, after some offline chats, I think this is better, since it
>> makes sure the ptr doesn't end up somewhere weird before we start the
>> calculations. This leaves the pointer as-is, but explicitly handles
>> the redzone on the offset instead, with no wrapping, etc:
>>
>> /* Find offset within object. */
>> offset = (ptr - page_address(page)) % s->size;
>>
>> +   /* Adjust for redzone and reject if within the redzone. */
>> +   if (s->flags & SLAB_RED_ZONE) {
>> +   if (offset < s->red_left_pad)
>> +   return s->name;
>> +   offset -= s->red_left_pad;
>> +   }
>> +
>> /* Allow address range falling entirely within object size. */
>> if (offset <= s->object_size && n <= s->object_size - offset)
>> return NULL;
>
> That fixes the case for me in kstrndup(), which allows the system to boot.

Ugh, no it doesn't, booted the wrong kernel.

I don't see the oops in strndup_user(), but instead get:

usercopy: kernel memory overwrite attempt detected to d3610028 
(cfq_io_cq) (88 bytes)
CPU: 11 PID: 1 Comm: systemd Not tainted 4.7.0-rc3-00098-g09d9556ae5d1-dirty #65
Call Trace:
[c001fb087bf0] [c09bdbe8] dump_stack+0xb0/0xf0 (unreliable)
[c001fb087c30] [c029cf44] __check_object_size+0x74/0x320
[c001fb087cb0] [c005d4d0] copy_from_user+0x60/0xd4
[c001fb087cf0] [c08b38f4] __get_filter+0x74/0x160
[c001fb087d30] [c08b408c] sk_attach_filter+0x2c/0xc0
[c001fb087d60] [c0871c34] sock_setsockopt+0x954/0xc00
[c001fb087dd0] [c086ac44] SyS_setsockopt+0x134/0x150
[c001fb087e30] [c0009260] system_call+0x38/0x108
Kernel panic - not syncing: Attempted to kill init! exitcode=0x0009

cheers
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev