linux-kernel - Re: [PATCH] powerpc/mm: Fix KUAP warning by providing copy_from_kernel_nofault

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0ede82c3-d4e9-6ce6-0590-6254272c3ae2@csgroup.eu>
Date:   Sat, 5 Dec 2020 10:56:56 +0100
From:   Christophe Leroy <christophe.leroy@...roup.eu>
To:     Christoph Hellwig <hch@....de>
Cc:     Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Paul Mackerras <paulus@...ba.org>,
        Michael Ellerman <mpe@...erman.id.au>, viro@...iv.linux.org.uk,
        akpm@...ux-foundation.org, linux-kernel@...r.kernel.org,
        linuxppc-dev@...ts.ozlabs.org, linux-mm@...ck.org,
        Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH] powerpc/mm: Fix KUAP warning by providing
 copy_from_kernel_nofault_allowed()



Le 05/12/2020 à 09:48, Christoph Hellwig a écrit :
> On Sat, Dec 05, 2020 at 08:43:06AM +0000, Christophe Leroy wrote:
>> Since commit c33165253492 ("powerpc: use non-set_fs based maccess
>> routines"), userspace access is not granted anymore when using
>> copy_from_kernel_nofault()
>>
>> However, kthread_probe_data() uses copy_from_kernel_nofault()
>> to check validity of pointers. When the pointer is NULL,
>> it points to userspace, leading to a KUAP fault and triggering
>> the following big hammer warning many times when you request
>> a sysrq "show task":
> 
> 
> 
>> To avoid that, copy_from_kernel_nofault_allowed() is used to check
>> whether the address is a valid kernel address. But the default
>> version of it returns true for any address.
>>
>> Provide a powerpc version of copy_from_kernel_nofault_allowed()
>> that returns false when the address is below TASK_USER_MAX,
>> so that copy_from_kernel_nofault() will return -ERANGE.
> 
> Looks good.  I wonder if we should just default to the TASK_SIZE_MAX
> check in  copy_from_kernel_nofault_allowed for architectures that select
> CONFIG_ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE?

Yes maybe that would be better.

Can you cook a patch an get it into 5.10 ?

Christophe

> 
>>
>> Reported-by: Qian Cai <qcai@...hat.com>
>> Fixes: c33165253492 ("powerpc: use non-set_fs based maccess routines")
>> Cc: Christoph Hellwig <hch@....de>
>> Cc: Al Viro <viro@...iv.linux.org.uk>
>> Signed-off-by: Christophe Leroy <christophe.leroy@...roup.eu>
>> ---
>> This issue was introduced in 5.10. I didn't mark it for stable, hopping it will go into 5.10-rc7
>> ---
>>   arch/powerpc/mm/Makefile  | 2 +-
>>   arch/powerpc/mm/maccess.c | 9 +++++++++
>>   2 files changed, 10 insertions(+), 1 deletion(-)
>>   create mode 100644 arch/powerpc/mm/maccess.c
>>
>> diff --git a/arch/powerpc/mm/Makefile b/arch/powerpc/mm/Makefile
>> index 5e147986400d..55b4a8bd408a 100644
>> --- a/arch/powerpc/mm/Makefile
>> +++ b/arch/powerpc/mm/Makefile
>> @@ -5,7 +5,7 @@
>>   
>>   ccflags-$(CONFIG_PPC64)	:= $(NO_MINIMAL_TOC)
>>   
>> -obj-y				:= fault.o mem.o pgtable.o mmap.o \
>> +obj-y				:= fault.o mem.o pgtable.o mmap.o maccess.o \
>>   				   init_$(BITS).o pgtable_$(BITS).o \
>>   				   pgtable-frag.o ioremap.o ioremap_$(BITS).o \
>>   				   init-common.o mmu_context.o drmem.o
>> diff --git a/arch/powerpc/mm/maccess.c b/arch/powerpc/mm/maccess.c
>> new file mode 100644
>> index 000000000000..56e97c0fb233
>> --- /dev/null
>> +++ b/arch/powerpc/mm/maccess.c
>> @@ -0,0 +1,9 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +
>> +#include <linux/uaccess.h>
>> +#include <linux/kernel.h>
>> +
>> +bool copy_from_kernel_nofault_allowed(const void *unsafe_src, size_t size)
>> +{
>> +	return (unsigned long)unsafe_src >= TASK_SIZE_MAX;
>> +}
>> -- 
>> 2.25.0
> ---end quoted text---
>