linux-kernel - Re: [patch 0/4] uaccess: Provide and use helpers for user masked access

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250817162945.64c943e1@pumpkin>
Date: Sun, 17 Aug 2025 16:29:45 +0100
From: David Laight <david.laight.linux@...il.com>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Thomas Gleixner <tglx@...utronix.de>, LKML
 <linux-kernel@...r.kernel.org>, Mathieu Desnoyers
 <mathieu.desnoyers@...icios.com>, Peter Zijlstra <peterz@...radead.org>,
 Darren Hart <dvhart@...radead.org>, Davidlohr Bueso <dave@...olabs.net>,
 André Almeida <andrealmeid@...lia.com>, x86@...nel.org,
 Alexander Viro <viro@...iv.linux.org.uk>, Christian Brauner
 <brauner@...nel.org>, Jan Kara <jack@...e.cz>,
 linux-fsdevel@...r.kernel.org
Subject: Re: [patch 0/4] uaccess: Provide and use helpers for user masked
 access

On Sun, 17 Aug 2025 07:00:17 -0700
Linus Torvalds <torvalds@...ux-foundation.org> wrote:

> On Sun, 17 Aug 2025 at 06:50, David Laight <david.laight.linux@...il.com> wrote:
> >
> > Linus didn't like it, but I've forgotten why.  
> 
> I think the reason I didn't love it is that it has a bit subtle
> semantics, and I think you just proved my point:

Just requiring the caller pass &user_ptr would make it more obvious.
The generated code (with 'src' -> *&src) will be the same.

> 
> > I'm also not convinced of the name.
> > There isn't any 'masking' involved, so it shouldn't be propagated.  
> 
> Sure there is. Look closer at that patch:
> 
> +       if (can_do_masked_user_access())                                \
> +               src = masked_user_access_begin(src);                    \
> 
> IOW, that macro changes the argument and masks it.

Except the change has never been a 'mask' in the traditional sense.
Neither the original cmp+sbb+or nor current cmp+cmov is really applying a mask.
I think the 'guard page' might even be the highest user page, so it isn't
even the case that kernel addresses get their low bits masked off.

The function could just be user_read_begin(void __user *addr, unsigned long len);
Although since it is the start of an unsafe_get_user() sequence perhaps
is should be unsafe_get_user_begin() ?

> 
> So it's actually really easy to use, but it's also really easy to miss
> that it does that.
> 
> We've done this before, and I have done it myself. The classic example
> is the whole "do_div()" macro that everybody hated because it did
> exactly the same thing

Divide is (well was, I think my zen5 has a fast divide) also slow enough that
I doubt it would have mattered.

- can you drop a 'must_check' on the div_u64() that people keep putting in
patches as a drop-in replacement for do_div()?

	David

> (we also have "save_flags()" etc that have this
> behavior).
> 
> So I don't love it - but I can't claim I've not done the same thing,
> and honestly, it does make it very easy to use, so when Thomas sent
> this series out I didn't speak out against it.
> 
>            Linus