From: David Laight <david.laight.linux@gmail.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: "Thomas Gleixner" <tglx@linutronix.de>,
LKML <linux-kernel@vger.kernel.org>,
"Mathieu Desnoyers" <mathieu.desnoyers@efficios.com>,
"Peter Zijlstra" <peterz@infradead.org>,
"Darren Hart" <dvhart@infradead.org>,
"Davidlohr Bueso" <dave@stgolabs.net>,
"André Almeida" <andrealmeid@igalia.com>,
x86@kernel.org, "Alexander Viro" <viro@zeniv.linux.org.uk>,
"Christian Brauner" <brauner@kernel.org>,
"Jan Kara" <jack@suse.cz>,
linux-fsdevel@vger.kernel.org
Subject: Re: [patch 0/4] uaccess: Provide and use helpers for user masked access
Date: Sun, 17 Aug 2025 16:29:45 +0100 [thread overview]
Message-ID: <20250817162945.64c943e1@pumpkin> (raw)
In-Reply-To: <CAHk-=wjsACUbLM-dAikbHzHBy6RFqyB1TdpHOMAJiGyNYM+FHA@mail.gmail.com>
On Sun, 17 Aug 2025 07:00:17 -0700
Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Sun, 17 Aug 2025 at 06:50, David Laight <david.laight.linux@gmail.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
next prev parent reply other threads:[~2025-08-17 15:29 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-08-13 15:57 [patch 0/4] uaccess: Provide and use helpers for user masked access Thomas Gleixner
2025-08-13 15:57 ` [patch 1/4] uaccess: Provide common helpers for masked user access Thomas Gleixner
2025-08-26 7:04 ` Christophe Leroy
2025-08-13 15:57 ` [patch 2/4] futex: Convert to get/put_user_masked_u32() Thomas Gleixner
2025-08-13 15:57 ` [patch 3/4] x86/futex: Use user_*_masked_begin() Thomas Gleixner
2025-08-26 7:09 ` Christophe Leroy
2025-08-13 15:57 ` [patch 4/4] select: Use user_read_masked_begin() Thomas Gleixner
2025-08-17 13:49 ` [patch 0/4] uaccess: Provide and use helpers for user masked access David Laight
2025-08-17 14:00 ` Linus Torvalds
2025-08-17 15:29 ` David Laight [this message]
2025-08-17 15:36 ` Linus Torvalds
2025-08-18 11:59 ` David Laight
2025-08-18 21:21 ` David Laight
2025-08-18 21:36 ` Linus Torvalds
2025-08-18 22:21 ` Al Viro
2025-08-18 23:00 ` Linus Torvalds
2025-08-19 0:39 ` Al Viro
2025-08-20 23:48 ` Al Viro
2025-08-21 7:45 ` Christian Brauner
2025-08-21 22:49 ` Al Viro
2025-08-19 2:39 ` Matthew Wilcox
2025-08-19 21:33 ` David Laight
2025-08-19 4:44 ` Thomas Weißschuh
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250817162945.64c943e1@pumpkin \
--to=david.laight.linux@gmail.com \
--cc=andrealmeid@igalia.com \
--cc=brauner@kernel.org \
--cc=dave@stgolabs.net \
--cc=dvhart@infradead.org \
--cc=jack@suse.cz \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mathieu.desnoyers@efficios.com \
--cc=peterz@infradead.org \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.org \
--cc=viro@zeniv.linux.org.uk \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).