Re: [PATCH v3 05/46] x86: asm: instrument usercopy in get_user() and __put_user_size()

[Arnd Bergmann]

On Tue, Apr 26, 2022 at 6:42 PM Alexander Potapenko <glider@xxxxxxxxxx> wrote:
> @@ -99,11 +100,13 @@ extern int __get_user_bad(void);
>         int __ret_gu;                                                   \
>         register __inttype(*(ptr)) __val_gu asm("%"_ASM_DX);            \
>         __chk_user_ptr(ptr);                                            \
> +       instrument_copy_from_user_before((void *)&(x), ptr, sizeof(*(ptr))); \
>         asm volatile("call __" #fn "_%P4"                               \
>                      : "=a" (__ret_gu), "=r" (__val_gu),                \
>                         ASM_CALL_CONSTRAINT                             \
>                      : "0" (ptr), "i" (sizeof(*(ptr))));                \
>         (x) = (__force __typeof__(*(ptr))) __val_gu;                    \
> +       instrument_copy_from_user_after((void *)&(x), ptr, sizeof(*(ptr)), 0); \

Isn't "ptr" the original pointer here? I think what happened with the
reported warning is that you get one output line for every instance this
is used in. There should probably be a

      __auto_type __ptr = (ptr);

at the beginning of the macro to ensure that 'ptr' is only evaluated once.

>>> arch/x86/kernel/signal.c:360:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void [noderef] __user *to @@     got unsigned long long [usertype] * @@

It would also make sense to add the missing __user annotation in this line, but
I suspect there are others like it in drivers.

      Arnd