On Mon, Jun 8, 2020 at 9:35 PM Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> From: Christoph Hellwig <hch@xxxxxx>
> Subject: x86: use non-set_fs based maccess routines
Ugh. I've seen this patch before, but only after I had applied it and
then pushed out the result did I notice that it has some nasty
behavior:
> + __get_user_size(*((type *)dst), (__force type __user *)src, \
Those casts of 'dst' and 'src' don't have the parentheses to make it
safe with random arguments. No such use exists right now, but these
aren't just some internal helper, they are used by code in
mm/maccess.c, and the arguments easily _could_ be something like
"ptr+offset", and then the pointer cast would do potentially very very
bad things.
I'm fixing it up, this is just a notice.
That's the trivial problem with that new macro, a more serious problem
is that I really think it should use
__inttype(*(src)) __gk_val;
__get_user_size(__gu_val, (__force type __user *)(src), ..
(dst) = (__force __typeof__(*(src)))__gk_val;
instead.
Because right now it does something horribly horribly wrong if the
type of 'src' is at all different from the type of dst, or either of
them is different from 'type'
But the fix to that is to remove the 'type' argument and the users, so
I'm not doing that part.
Christoph? I'm not seeing why you insisted on making
__get_kernel_nofault different from unsafe_get_user().
Because as far as I can tell, every single difference is basically a
bug waiting to happen.
Linus
