On Fri, 18 Jul 2025 at 14:49, Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Fri, 18 Jul 2025 at 14:34, H. Peter Anvin <hpa@xxxxxxxxx> wrote:
> >
> > - __auto_type __pu_ptr = (ptr); \
> > + auto __pu_ptr = (ptr); \
> > typeof(*__pu_ptr) __pu_val = (typeof(*__pu_ptr))(x); \
>
> But that second case obviously is exactly the "auto type" case, just
> written using __typeof__.
Actually, looking at it, I actually think the NIOS2 header is a bit
buggy here, exactly because it should *not* have that cast to force
the types the same.
It's the exact same situation that on x86 is inside
do_put_user_call(), and on x86 uses that
__typeof__(*(ptr)) __x = (x); /* eval x once */
pattern instead: we don't want a cast, because we actually want just
the implicit type conversions, and a warning if the types aren't
compatible. Writing things to user space is still supposed to catch
type safety issues.
So having that '(typeof(*__pu_ptr))' cast of the value of '(x)' is
actually wrong, because it will silently (for example) convert a
pointer into a 'unsigned long' or vice versa, and __put_user() just
shouldn't do that.
If the user access is to a 'unsigned long __user *' location, the
kernel shouldn't be writing pointers into it.
Do we care? No. This is obviously nios2-specific, and the x86 version
will catch any generic mis-uses where somebody would try to
'put_user()' the wrong type.
And any "auto" conversion wouldn't change the bug anyway. But I
thought I'd mention it since it started bothering me and I went and
looked closer at that case I quoted.
And while looking at this, I think we have a similar mis-feature / bug
on x86 too: the unsafe_put_user() macro does exactly that cast:
#define unsafe_put_user(x, ptr, label) \
__put_user_size((__typeof__(*(ptr)))(x), ..
and I think that cast is wrong.
I wonder if it's actively hiding some issue with unsafe_put_user(), or
if I'm just missing something.
Linus
