[PATCH v8 16/30] powerpc: Define and use __get_user_instr{, inatomic}()

[Michael Ellerman]

Jordan Niethe <jniethe5 at gmail.com> writes:
> Define specific __get_user_instr() and __get_user_instr_inatomic()
> macros for reading instructions from user space.

At least for fix_alignment() we could be coming from the kernel, not
sure about the other cases.

I can tweak the change log.

> diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
> index 2f500debae21..c0a35e4586a5 100644
> --- a/arch/powerpc/include/asm/uaccess.h
> +++ b/arch/powerpc/include/asm/uaccess.h
> @@ -105,6 +105,11 @@ static inline int __access_ok(unsigned long addr, unsigned long size,
>  #define __put_user_inatomic(x, ptr) \
>  	__put_user_nosleep((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr)))
>  
> +#define __get_user_instr(x, ptr) \
> +	__get_user_nocheck((x).val, (u32 *)(ptr), sizeof(u32), true)
> +
> +#define __get_user_instr_inatomic(x, ptr) \
> +	__get_user_nosleep((x).val, (u32 *)(ptr), sizeof(u32))

I'm not super keen on adding new __ versions, which lack the access_ok()
check, but I guess we have to.

> diff --git a/arch/powerpc/kernel/vecemu.c b/arch/powerpc/kernel/vecemu.c
> index 3dd70eeb10c5..60ed5aea8d4e 100644
> --- a/arch/powerpc/kernel/vecemu.c
> +++ b/arch/powerpc/kernel/vecemu.c
> @@ -266,7 +266,7 @@ int emulate_altivec(struct pt_regs *regs)
>  	unsigned int va, vb, vc, vd;
>  	vector128 *vrs;
>  
> -	if (get_user(instr.val, (unsigned int __user *)regs->nip))
> +	if (__get_user_instr(instr, (void __user *)regs->nip))
>  		return -EFAULT;

That drops the access_ok() check, which is not OK, at least without a
reasonable justification.

Given it's regs->nip I guess it should be safe, but it should still be
called out. Or preferably switched to __get_user() in a precursor patch.

cheers