[PATCH 1/4] powerpc/64s: implement probe_kernel_read/write without touching AMR

[Nicholas Piggin]

Christophe Leroy's on March 27, 2020 5:13 pm:
> 
> 
> Le 27/03/2020 à 08:02, Nicholas Piggin a écrit :
>> There is no need to allow user accesses when probing kernel addresses.
>> 
>> Signed-off-by: Nicholas Piggin <npiggin at gmail.com>
>> ---
>>   arch/powerpc/include/asm/uaccess.h | 25 ++++++++++-----
>>   arch/powerpc/lib/Makefile          |  2 +-
>>   arch/powerpc/lib/uaccess.c         | 50 ++++++++++++++++++++++++++++++
>>   3 files changed, 68 insertions(+), 9 deletions(-)
>>   create mode 100644 arch/powerpc/lib/uaccess.c
>> 
> 
> [...]
> 
>> diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
>> index b8de3be10eb4..a15060b5008e 100644
>> --- a/arch/powerpc/lib/Makefile
>> +++ b/arch/powerpc/lib/Makefile
>> @@ -36,7 +36,7 @@ extra-$(CONFIG_PPC64)	+= crtsavres.o
>>   endif
>>   
>>   obj-$(CONFIG_PPC_BOOK3S_64) += copyuser_power7.o copypage_power7.o \
>> -			       memcpy_power7.o
>> +			       memcpy_power7.o uaccess.o
> 
> Why only book3s/64 ? It applies to the 8xx and book3s/32 as well, I 
> think it should just be for all powerpc.

Okay I can do that.

>>   
>>   obj64-y	+= copypage_64.o copyuser_64.o mem_64.o hweight_64.o \
>>   	   memcpy_64.o memcpy_mcsafe_64.o
>> diff --git a/arch/powerpc/lib/uaccess.c b/arch/powerpc/lib/uaccess.c
>> new file mode 100644
>> index 000000000000..0057ab52d6fe
>> --- /dev/null
>> +++ b/arch/powerpc/lib/uaccess.c
>> @@ -0,0 +1,50 @@
>> +#include <linux/mm.h>
>> +#include <linux/uaccess.h>
>> +
>> +static __always_inline long
>> +probe_read_common(void *dst, const void __user *src, size_t size)
>> +{
>> +	long ret;
>> +
>> +	pagefault_disable();
>> +	ret = raw_copy_from_user_allowed(dst, src, size);
>> +	pagefault_enable();
>> +
>> +	return ret ? -EFAULT : 0;
>> +}
>> +
>> +static __always_inline long
>> +probe_write_common(void __user *dst, const void *src, size_t size)
>> +{
>> +	long ret;
>> +
>> +	pagefault_disable();
>> +	ret = raw_copy_to_user_allowed(dst, src, size);
>> +	pagefault_enable();
>> +
>> +	return ret ? -EFAULT : 0;
>> +}
>> +
>> +long probe_kernel_read(void *dst, const void *src, size_t size)
>> +{
>> +	long ret;
>> +	mm_segment_t old_fs = get_fs();
>> +
>> +	set_fs(KERNEL_DS);
>> +	ret = probe_read_common(dst, (__force const void __user *)src, size);
> 
> I think you should squash probe_read_common() here, having it separated 
> is a lot of lines for no added value. It also may make people believe it 
> overwrites the generic probe_read_common()

Yeah I'll see how that looks.

Thanks,
Nick