Re: [PATCH 2/4] riscv: uaccess: use input constraints for ptr of __put_user

Arnd Bergmann
2024-06-25T05:55:07Z

On Tue, Jun 25, 2024, at 06:04, Jisheng Zhang wrote:
> I believe the output constraints "=m" is not necessary, because
> the instruction itself is "write", we don't need the compiler
> to "write" for us. So tell compiler we read from memory instead
> of writing.
>
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>

I think this is a bit too confusing: clearly there is no
read access from the __user pointer, so what you add in here
is not correct. There also needs to be a code comment about
why you do it this way, as it's not clear that this is
a workaround for old compilers without
CONFIG_CC_HAS_ASM_GOTO_OUTPUT.

> index 09d4ca37522c..84b084e388a7 100644
> --- a/arch/riscv/include/asm/uaccess.h
> +++ b/arch/riscv/include/asm/uaccess.h
> @@ -186,11 +186,11 @@ do { \
> __typeof__(*(ptr)) __x = x; \
> __asm__ __volatile__ ( \
> "1:\n" \
> - " " insn " %z2, %1\n" \
> + " " insn " %z1, %2\n" \
> "2:\n" \
> _ASM_EXTABLE_UACCESS_ERR(1b, 2b, %0) \
> - : "+r" (err), "=m" (*(ptr)) \
> - : "rJ" (__x)); \
> + : "+r" (err) \
> + : "rJ" (__x), "m"(*(ptr))); \
> } while (0)
>

I suspect this could just be a "r" constraint instead of
"m", treating the __user pointer as a plain integer.

For kernel pointers, using "m" and "=m" constraints
correctly is necessary since gcc will often access the
same data from C code as well. For __user pointers, we
can probably get away without it since no C code is
ever allowed to just dereference them. If you do that,
you may want to have the same thing in the __get_user
side.

Arnd