All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC][PATCH] get rid of bogus __user in struct xattr_args::value
@ 2025-12-16  8:19 Al Viro
  2025-12-24 12:53 ` Christian Brauner
  0 siblings, 1 reply; 2+ messages in thread
From: Al Viro @ 2025-12-16  8:19 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-fsdevel, Christian Brauner, Arnd Bergmann

	The first member of struct xattr_args is declared as
	__aligned_u64 __user value;
which makes no sense whatsoever; __user is a qualifier and what that
declaration says is "all struct xattr_args instances have .value
_stored_ in user address space, no matter where the rest of the
structure happens to be".

	Something like "int __user *p" stands for "value of p is a pointer
to an instance of int that happens to live in user address space"; it
says nothing about location of p itself, just as const char *p declares a
pointer to unmodifiable char rather than an unmodifiable pointer to char.

	With xattr_args the intent clearly had been "the 64bit value
represents a _pointer_ to object in user address space", but __user has
nothing to do with that.  All it gets us is a couple of bogus warnings
in fs/xattr.c where (userland) instance of xattr_args is copied to local
variable of that type (in kernel address space), followed by access
to its members.  Since we've told sparse that args.value must somehow be
located in userland memory, we get warned that looking at that 64bit
unsigned integer (in a variable already on kernel stack) is not allowed.

	Note that sparse has no way to express "this integer shall never
be cast into a pointer to be dereferenced directly" and I don't see any
way to assign a sane semantics to that.  In any case, __user is not it.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---

PS: one variant we might implement in sparse is a type attribute similar
to __bitwise__ (__affine__, perhaps?) such that
T + integer => T, as long as integer type promotes to base type of T
T - integer => ditto
T - T => base type of T
T <comparison> T => int
and perhaps
T & integer => integer, for the sake of things like alignment checks, etc.
any other arithmetics on T => error
cast or conversion from T to anything other than qualified T => error
cast or conversion to T from anything other than qualified T => error
force-cast to or from T => allowed
Then we could have something like __encoded_uptr declared as __u64 with
such attribute in addition to __aligned(8) we already have on __aligned_u64,
with
static inline void __user *decode_uptr(__encoded_uptr v)
{
	return (__force void __user *)v;
}
with that helper used instead of u64_to_user_ptr() fs/xattr.c currently uses
for those.  It's not that hard to implement, but I'm not sure how useful would
that be...

diff --git a/include/uapi/linux/xattr.h b/include/uapi/linux/xattr.h
index c7c85bb504ba..2e5aef48fa7e 100644
--- a/include/uapi/linux/xattr.h
+++ b/include/uapi/linux/xattr.h
@@ -23,7 +23,7 @@
 #define XATTR_REPLACE	0x2	/* set value, fail if attr does not exist */
 
 struct xattr_args {
-	__aligned_u64 __user value;
+	__aligned_u64 value;
 	__u32 size;
 	__u32 flags;
 };

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [RFC][PATCH] get rid of bogus __user in struct xattr_args::value
  2025-12-16  8:19 [RFC][PATCH] get rid of bogus __user in struct xattr_args::value Al Viro
@ 2025-12-24 12:53 ` Christian Brauner
  0 siblings, 0 replies; 2+ messages in thread
From: Christian Brauner @ 2025-12-24 12:53 UTC (permalink / raw)
  To: Al Viro; +Cc: Christian Brauner, linux-fsdevel, Arnd Bergmann, Linus Torvalds

On Tue, 16 Dec 2025 08:19:39 +0000, Al Viro wrote:
> 	The first member of struct xattr_args is declared as
> 	__aligned_u64 __user value;
> which makes no sense whatsoever; __user is a qualifier and what that
> declaration says is "all struct xattr_args instances have .value
> _stored_ in user address space, no matter where the rest of the
> structure happens to be".
> 
> [...]

Applied to the vfs.fixes branch of the vfs/vfs.git tree.
Patches in the vfs.fixes branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.fixes

[1/1] get rid of bogus __user in struct xattr_args::value
      https://git.kernel.org/vfs/vfs/c/3dd57ddec9e3

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-12-24 12:53 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-16  8:19 [RFC][PATCH] get rid of bogus __user in struct xattr_args::value Al Viro
2025-12-24 12:53 ` Christian Brauner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.