* Re: [PATCH] uaccess: Fix build of scoped user access with const pointer
@ 2026-03-01 20:01 ` Linus Torvalds
0 siblings, 0 replies; 10+ messages in thread
From: Linus Torvalds @ 2026-03-01 20:01 UTC (permalink / raw)
To: Christophe Leroy (CS GROUP)
Cc: Thomas Gleixner, LKML, Christophe Leroy, Mathieu Desnoyers,
Andrew Cooper, David Laight, kernel test robot, Russell King,
linux-arm-kernel, x86, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, linuxppc-dev, Paul Walmsley, Palmer Dabbelt,
linux-riscv, Heiko Carstens, Christian Borntraeger, Sven Schnelle,
linux-s390, Julia Lawall, Nicolas Palix, Peter Zijlstra,
Darren Hart, Davidlohr Bueso, Andre Almeida, Alexander Viro,
Christian Brauner, Jan Kara, linux-fsdevel
On Sun, 1 Mar 2026 at 11:34, Christophe Leroy (CS GROUP)
<chleroy@kernel.org> wrote:
>
> - for (void __user *_tmpptr = __scoped_user_access_begin(mode, uptr, size, elbl); \
> + for (void __user *_tmpptr = (void __user *) \
> + __scoped_user_access_begin(mode, uptr, size, elbl); \
Why are you casting this return value? Wouldn't it be a lot better to
just make the types be the CORRECT ones?
I didn't test this, so maybe I'm missing something, but why isn't that
just doing
for (auto _tmpptr = __scoped_user_access_begin(mode, uptr,
size, elbl); \
instead? No cast, just a "use the right type automatically".
That macro actually does something similar just a few lines later, in
that the innermost loop uses
for (const typeof(uptr) uptr = _tmpptr; !done; done = true)
which picks up the type automatically from the argument (and then it
uses the argument both for the type and name, which is horrendously
confusing, but that's a separate thing).
Does that simple "auto" approach break something else?
Linus
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] uaccess: Fix build of scoped user access with const pointer
2026-03-01 20:01 ` Linus Torvalds
@ 2026-03-01 21:59 ` David Laight
-1 siblings, 0 replies; 10+ messages in thread
From: David Laight @ 2026-03-01 21:59 UTC (permalink / raw)
To: Linus Torvalds
Cc: Christophe Leroy (CS GROUP), Thomas Gleixner, LKML,
Christophe Leroy, Mathieu Desnoyers, Andrew Cooper,
kernel test robot, Russell King, linux-arm-kernel, x86,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
linuxppc-dev, Paul Walmsley, Palmer Dabbelt, linux-riscv,
Heiko Carstens, Christian Borntraeger, Sven Schnelle, linux-s390,
Julia Lawall, Nicolas Palix, Peter Zijlstra, Darren Hart,
Davidlohr Bueso, Andre Almeida, Alexander Viro, Christian Brauner,
Jan Kara, linux-fsdevel
On Sun, 1 Mar 2026 12:01:08 -0800
Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Sun, 1 Mar 2026 at 11:34, Christophe Leroy (CS GROUP)
> <chleroy@kernel.org> wrote:
> >
> > - for (void __user *_tmpptr = __scoped_user_access_begin(mode, uptr, size, elbl); \
> > + for (void __user *_tmpptr = (void __user *) \
> > + __scoped_user_access_begin(mode, uptr, size, elbl); \
>
> Why are you casting this return value? Wouldn't it be a lot better to
> just make the types be the CORRECT ones?
>
> I didn't test this, so maybe I'm missing something, but why isn't that
> just doing
>
> for (auto _tmpptr = __scoped_user_access_begin(mode, uptr,
> size, elbl); \
>
> instead? No cast, just a "use the right type automatically".
>
> That macro actually does something similar just a few lines later, in
> that the innermost loop uses
>
> for (const typeof(uptr) uptr = _tmpptr; !done; done = true)
>
> which picks up the type automatically from the argument (and then it
> uses the argument both for the type and name, which is horrendously
> confusing, but that's a separate thing).
>
> Does that simple "auto" approach break something else?
This is what I needed to do:
(Note that is pre-dates 'auto', but it should work.)
Send at 21:56 on dec 20.
If a 'const struct foo __user *ptr' is used for the address passed
to scoped_user_read_access() then you get a warning/error
uaccess.h:691:1: error: initialization discards 'const' qualifier
from pointer target type [-Werror=discarded-qualifiers]
for the
void __user *_tmpptr = __scoped_user_access_begin(mode, uptr, size, elbl)
assignment.
Fix by using typeof(uptr) in that assignment and changing the 'read' functions
to use 'const void __user *ptr' rather than 'void __user *ptr'.
Fixes: e497310b4ffb "(uaccess: Provide scoped user access regions)"
Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
include/linux/uaccess.h | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
index 1f3804245c06..c5d5f2d395bc 100644
--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -650,32 +650,32 @@ static inline void user_access_restore(unsigned long flags) { }
#define user_rw_access_end() user_access_end()
/* Scoped user access */
-#define USER_ACCESS_GUARD(_mode) \
-static __always_inline void __user * \
-class_user_##_mode##_begin(void __user *ptr) \
+#define USER_ACCESS_GUARD(_mode, type) \
+static __always_inline type __user * \
+class_user_##_mode##_begin(type __user *ptr) \
{ \
return ptr; \
} \
\
static __always_inline void \
-class_user_##_mode##_end(void __user *ptr) \
+class_user_##_mode##_end(type __user *ptr) \
{ \
user_##_mode##_access_end(); \
} \
\
-DEFINE_CLASS(user_ ##_mode## _access, void __user *, \
+DEFINE_CLASS(user_ ##_mode## _access, type __user *, \
class_user_##_mode##_end(_T), \
- class_user_##_mode##_begin(ptr), void __user *ptr) \
+ class_user_##_mode##_begin(ptr), type __user *ptr) \
\
static __always_inline class_user_##_mode##_access_t \
-class_user_##_mode##_access_ptr(void __user *scope) \
+class_user_##_mode##_access_ptr(type __user *scope) \
{ \
return scope; \
}
-USER_ACCESS_GUARD(read)
-USER_ACCESS_GUARD(write)
-USER_ACCESS_GUARD(rw)
+USER_ACCESS_GUARD(read, const void)
+USER_ACCESS_GUARD(write, void)
+USER_ACCESS_GUARD(rw, void)
#undef USER_ACCESS_GUARD
/**
@@ -752,7 +752,7 @@ USER_ACCESS_GUARD(rw)
*/
#define __scoped_user_access(mode, uptr, size, elbl) \
for (bool done = false; !done; done = true) \
- for (void __user *_tmpptr = __scoped_user_access_begin(mode, uptr, size, elbl); \
+ for (typeof(uptr) _tmpptr = __scoped_user_access_begin(mode, uptr, size, elbl); \
!done; done = true) \
for (CLASS(user_##mode##_access, scope)(_tmpptr); !done; done = true) \
/* Force modified pointer usage within the scope */ \
--
2.39.5
David
>
> Linus
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH] uaccess: Fix build of scoped user access with const pointer
@ 2026-03-01 21:59 ` David Laight
0 siblings, 0 replies; 10+ messages in thread
From: David Laight @ 2026-03-01 21:59 UTC (permalink / raw)
To: Linus Torvalds
Cc: Christophe Leroy (CS GROUP), Thomas Gleixner, LKML,
Christophe Leroy, Mathieu Desnoyers, Andrew Cooper,
kernel test robot, Russell King, linux-arm-kernel, x86,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
linuxppc-dev, Paul Walmsley, Palmer Dabbelt, linux-riscv,
Heiko Carstens, Christian Borntraeger, Sven Schnelle, linux-s390,
Julia Lawall, Nicolas Palix, Peter Zijlstra, Darren Hart,
Davidlohr Bueso, Andre Almeida, Alexander Viro, Christian Brauner,
Jan Kara, linux-fsdevel
On Sun, 1 Mar 2026 12:01:08 -0800
Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Sun, 1 Mar 2026 at 11:34, Christophe Leroy (CS GROUP)
> <chleroy@kernel.org> wrote:
> >
> > - for (void __user *_tmpptr = __scoped_user_access_begin(mode, uptr, size, elbl); \
> > + for (void __user *_tmpptr = (void __user *) \
> > + __scoped_user_access_begin(mode, uptr, size, elbl); \
>
> Why are you casting this return value? Wouldn't it be a lot better to
> just make the types be the CORRECT ones?
>
> I didn't test this, so maybe I'm missing something, but why isn't that
> just doing
>
> for (auto _tmpptr = __scoped_user_access_begin(mode, uptr,
> size, elbl); \
>
> instead? No cast, just a "use the right type automatically".
>
> That macro actually does something similar just a few lines later, in
> that the innermost loop uses
>
> for (const typeof(uptr) uptr = _tmpptr; !done; done = true)
>
> which picks up the type automatically from the argument (and then it
> uses the argument both for the type and name, which is horrendously
> confusing, but that's a separate thing).
>
> Does that simple "auto" approach break something else?
This is what I needed to do:
(Note that is pre-dates 'auto', but it should work.)
Send at 21:56 on dec 20.
If a 'const struct foo __user *ptr' is used for the address passed
to scoped_user_read_access() then you get a warning/error
uaccess.h:691:1: error: initialization discards 'const' qualifier
from pointer target type [-Werror=discarded-qualifiers]
for the
void __user *_tmpptr = __scoped_user_access_begin(mode, uptr, size, elbl)
assignment.
Fix by using typeof(uptr) in that assignment and changing the 'read' functions
to use 'const void __user *ptr' rather than 'void __user *ptr'.
Fixes: e497310b4ffb "(uaccess: Provide scoped user access regions)"
Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
include/linux/uaccess.h | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
index 1f3804245c06..c5d5f2d395bc 100644
--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -650,32 +650,32 @@ static inline void user_access_restore(unsigned long flags) { }
#define user_rw_access_end() user_access_end()
/* Scoped user access */
-#define USER_ACCESS_GUARD(_mode) \
-static __always_inline void __user * \
-class_user_##_mode##_begin(void __user *ptr) \
+#define USER_ACCESS_GUARD(_mode, type) \
+static __always_inline type __user * \
+class_user_##_mode##_begin(type __user *ptr) \
{ \
return ptr; \
} \
\
static __always_inline void \
-class_user_##_mode##_end(void __user *ptr) \
+class_user_##_mode##_end(type __user *ptr) \
{ \
user_##_mode##_access_end(); \
} \
\
-DEFINE_CLASS(user_ ##_mode## _access, void __user *, \
+DEFINE_CLASS(user_ ##_mode## _access, type __user *, \
class_user_##_mode##_end(_T), \
- class_user_##_mode##_begin(ptr), void __user *ptr) \
+ class_user_##_mode##_begin(ptr), type __user *ptr) \
\
static __always_inline class_user_##_mode##_access_t \
-class_user_##_mode##_access_ptr(void __user *scope) \
+class_user_##_mode##_access_ptr(type __user *scope) \
{ \
return scope; \
}
-USER_ACCESS_GUARD(read)
-USER_ACCESS_GUARD(write)
-USER_ACCESS_GUARD(rw)
+USER_ACCESS_GUARD(read, const void)
+USER_ACCESS_GUARD(write, void)
+USER_ACCESS_GUARD(rw, void)
#undef USER_ACCESS_GUARD
/**
@@ -752,7 +752,7 @@ USER_ACCESS_GUARD(rw)
*/
#define __scoped_user_access(mode, uptr, size, elbl) \
for (bool done = false; !done; done = true) \
- for (void __user *_tmpptr = __scoped_user_access_begin(mode, uptr, size, elbl); \
+ for (typeof(uptr) _tmpptr = __scoped_user_access_begin(mode, uptr, size, elbl); \
!done; done = true) \
for (CLASS(user_##mode##_access, scope)(_tmpptr); !done; done = true) \
/* Force modified pointer usage within the scope */ \
--
2.39.5
David
>
> Linus
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] uaccess: Fix build of scoped user access with const pointer
2026-03-01 20:01 ` Linus Torvalds
@ 2026-03-01 22:16 ` David Laight
-1 siblings, 0 replies; 10+ messages in thread
From: David Laight @ 2026-03-01 22:16 UTC (permalink / raw)
To: Linus Torvalds
Cc: Christophe Leroy (CS GROUP), Thomas Gleixner, LKML,
Christophe Leroy, Mathieu Desnoyers, Andrew Cooper,
kernel test robot, Russell King, linux-arm-kernel, x86,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
linuxppc-dev, Paul Walmsley, Palmer Dabbelt, linux-riscv,
Heiko Carstens, Christian Borntraeger, Sven Schnelle, linux-s390,
Julia Lawall, Nicolas Palix, Peter Zijlstra, Darren Hart,
Davidlohr Bueso, Andre Almeida, Alexander Viro, Christian Brauner,
Jan Kara, linux-fsdevel
On Sun, 1 Mar 2026 12:01:08 -0800
Linus Torvalds <torvalds@linux-foundation.org> wrote:
I also added to compiler.h :
+/*
+ * Sometimes a #define needs to declare a variable that is scoped
+ * to the statement that follows without having mismatched {}.
+ * with (int x = expression) {
+ * statements
+ * }
+ * is the same as:
+ * {
+ * int x = expression;
+ * statements
+ * }
+ * but lets it all be hidden from the call site, eg:
+ * frobnicate(args) {
+ * statements
+ * }
+ * Only a single variable can be defined, and_with() allows extra ones
+ * without adding an additional outer loop.
+ *
+ * The controlled scope can be terminated using break, continue or goto.
+ */
+#define with(declaration) \
+ for (bool _with_done = false; !_with_done; _with_done = true) \
+ and_with (declaration)
+#define and_with(declaration) \
+ for (declaration; !_with_done; _with_done = true)
+
So that you get:
#define __scoped_user_access(mode, uptr, size, elbl) \
with (auto _tmpptr = __scoped_user_access_begin(mode, uptr, size, elbl)) \
and_with (CLASS(user_##mode##_access, scope)(_tmpptr)) \
/* Force modified pointer usage within the scope */ \
and_with (const auto uptr = _tmpptr)
The next patch did:
- and_with (const typeof(uptr) uptr = _tmpptr)
+ __diag_push() __diag_ignore_all("-Wshadow", "uptr is readonly copy") \
+ and_with (const typeof(uptr) uptr = _tmpptr) \
+ __diag_pop()
I'll update (to use auto as above) and resend.
David
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] uaccess: Fix build of scoped user access with const pointer
@ 2026-03-01 22:16 ` David Laight
0 siblings, 0 replies; 10+ messages in thread
From: David Laight @ 2026-03-01 22:16 UTC (permalink / raw)
To: Linus Torvalds
Cc: Christophe Leroy (CS GROUP), Thomas Gleixner, LKML,
Christophe Leroy, Mathieu Desnoyers, Andrew Cooper,
kernel test robot, Russell King, linux-arm-kernel, x86,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
linuxppc-dev, Paul Walmsley, Palmer Dabbelt, linux-riscv,
Heiko Carstens, Christian Borntraeger, Sven Schnelle, linux-s390,
Julia Lawall, Nicolas Palix, Peter Zijlstra, Darren Hart,
Davidlohr Bueso, Andre Almeida, Alexander Viro, Christian Brauner,
Jan Kara, linux-fsdevel
On Sun, 1 Mar 2026 12:01:08 -0800
Linus Torvalds <torvalds@linux-foundation.org> wrote:
I also added to compiler.h :
+/*
+ * Sometimes a #define needs to declare a variable that is scoped
+ * to the statement that follows without having mismatched {}.
+ * with (int x = expression) {
+ * statements
+ * }
+ * is the same as:
+ * {
+ * int x = expression;
+ * statements
+ * }
+ * but lets it all be hidden from the call site, eg:
+ * frobnicate(args) {
+ * statements
+ * }
+ * Only a single variable can be defined, and_with() allows extra ones
+ * without adding an additional outer loop.
+ *
+ * The controlled scope can be terminated using break, continue or goto.
+ */
+#define with(declaration) \
+ for (bool _with_done = false; !_with_done; _with_done = true) \
+ and_with (declaration)
+#define and_with(declaration) \
+ for (declaration; !_with_done; _with_done = true)
+
So that you get:
#define __scoped_user_access(mode, uptr, size, elbl) \
with (auto _tmpptr = __scoped_user_access_begin(mode, uptr, size, elbl)) \
and_with (CLASS(user_##mode##_access, scope)(_tmpptr)) \
/* Force modified pointer usage within the scope */ \
and_with (const auto uptr = _tmpptr)
The next patch did:
- and_with (const typeof(uptr) uptr = _tmpptr)
+ __diag_push() __diag_ignore_all("-Wshadow", "uptr is readonly copy") \
+ and_with (const typeof(uptr) uptr = _tmpptr) \
+ __diag_pop()
I'll update (to use auto as above) and resend.
David
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 10+ messages in thread