On Mon, 2025-10-06 at 13:51 -0700, Linus Torvalds wrote: > Danger Will Robinson: hostfs has odd duplicate copies of all these, including > a > > #define HOSTFS_ATTR_ATTR_FLAG 1024 > > of that no-longer existing flag. > > But hostfs doesn't use ATTR_FORCE (aka HOSTFS_ATTR_FORCE), so > switching those two bits around wouldn't affect it either, even if you > were to have a version mismatch between the client and host when doing > UML (which I don't know > > Adding Christian to the participants list, because I did *not* do that > cleanup thing myself, because I'm slightly worried that I'm missing > something. But it would seem to be a good thing to do just to have the > numbering make more sense, and Christian is probably the right person.
That indeed looks messy in hostfs; I'm not really all _that_ familiar with all the details of it, but the stated reason is that it needs to have the defines in kernel and user code. That doesn't mean it's between the host and guest kernels, it's just between code built for "userspace" and code built for "kernel", both of which go into the UML linux "guest" binary. IOW, it's just for communication between hostfs_kern.c and hostfs_user.c, which nobody else needs to care about. And as long as hostfs doesn't propagate CTIME_SET from the host to the guest, it doesn't need a HOSTFS_ATTR_CTIME_SET. No idea why HOSTFS_ATTR_ATTR_FLAG was even defined, it's ancient anyway. However ... it looks like hostfs_kern.c is using ATTR_* in some places, and hostfs_user.c is using HOSTFS_ATTR_*, so it looks like right now these *do* need to match. Given that, we should just generate them via asm-offsets.h, like we do for other constants with the property of being needed on both sides but defined in places that cannot be included into user-side code, like this: From: Johannes Berg <[email protected]> Date: Mon, 6 Oct 2025 23:14:36 +0200 Subject: [PATCH] um/hostfs: define HOSTFS_ATTR_* via asm-offsets The HOSTFS_ATTR_* values were meant to be standalone for communication between hostfs's kernel and user code parts. However, it's easy to forget that HOSTFS_ATTR_* should be used even on the kernel side, and that wasn't consistently done. As a result, the values need to match ATTR_* values, which is not useful to maintain by hand. Instead, generate them via asm-offsets like other constants that UML needs in user-side code that aren't otherwise available in any header files that can be included there. Signed-off-by: Johannes Berg <[email protected]> --- arch/um/include/shared/common-offsets.h | 10 +++++++ arch/x86/um/shared/sysdep/kernel-offsets.h | 1 + fs/hostfs/hostfs.h | 34 +--------------------- 3 files changed, 12 insertions(+), 33 deletions(-) diff --git a/arch/um/include/shared/common-offsets.h b/arch/um/include/shared/common-offsets.h index 8ca66a1918c3..fcec75a93e0c 100644 --- a/arch/um/include/shared/common-offsets.h +++ b/arch/um/include/shared/common-offsets.h @@ -18,3 +18,13 @@ DEFINE(UM_NSEC_PER_USEC, NSEC_PER_USEC); DEFINE(UM_KERN_GDT_ENTRY_TLS_ENTRIES, GDT_ENTRY_TLS_ENTRIES); DEFINE(UM_SECCOMP_ARCH_NATIVE, SECCOMP_ARCH_NATIVE); + +DEFINE(HOSTFS_ATTR_MODE, ATTR_MODE); +DEFINE(HOSTFS_ATTR_UID, ATTR_UID); +DEFINE(HOSTFS_ATTR_GID, ATTR_GID); +DEFINE(HOSTFS_ATTR_SIZE, ATTR_SIZE); +DEFINE(HOSTFS_ATTR_ATIME, ATTR_ATIME); +DEFINE(HOSTFS_ATTR_MTIME, ATTR_MTIME); +DEFINE(HOSTFS_ATTR_CTIME, ATTR_CTIME); +DEFINE(HOSTFS_ATTR_ATIME_SET, ATTR_ATIME_SET); +DEFINE(HOSTFS_ATTR_MTIME_SET, ATTR_MTIME_SET); diff --git a/arch/x86/um/shared/sysdep/kernel-offsets.h b/arch/x86/um/shared/sysdep/kernel-offsets.h index 6fd1ed400399..ee6b44ef2217 100644 --- a/arch/x86/um/shared/sysdep/kernel-offsets.h +++ b/arch/x86/um/shared/sysdep/kernel-offsets.h @@ -5,6 +5,7 @@ #include <linux/crypto.h> #include <linux/kbuild.h> #include <linux/audit.h> +#include <linux/fs.h> #include <asm/mman.h> #include <asm/seccomp.h> diff --git a/fs/hostfs/hostfs.h b/fs/hostfs/hostfs.h index 15b2f094d36e..aa02599b770f 100644 --- a/fs/hostfs/hostfs.h +++ b/fs/hostfs/hostfs.h @@ -3,40 +3,8 @@ #define __UM_FS_HOSTFS #include <os.h> +#include <generated/asm-offsets.h> -/* - * These are exactly the same definitions as in fs.h, but the names are - * changed so that this file can be included in both kernel and user files. - */ - -#define HOSTFS_ATTR_MODE 1 -#define HOSTFS_ATTR_UID 2 -#define HOSTFS_ATTR_GID 4 -#define HOSTFS_ATTR_SIZE 8 -#define HOSTFS_ATTR_ATIME 16 -#define HOSTFS_ATTR_MTIME 32 -#define HOSTFS_ATTR_CTIME 64 -#define HOSTFS_ATTR_ATIME_SET 128 -#define HOSTFS_ATTR_MTIME_SET 256 - -/* This one is unused by hostfs. */ -#define HOSTFS_ATTR_FORCE 512 /* Not a change, but a change it */ -#define HOSTFS_ATTR_ATTR_FLAG 1024 - -/* - * If you are very careful, you'll notice that these two are missing: - * - * #define ATTR_KILL_SUID 2048 - * #define ATTR_KILL_SGID 4096 - * - * and this is because they were added in 2.5 development. - * Actually, they are not needed by most ->setattr() methods - they are set by - * callers of notify_change() to notify that the setuid/setgid bits must be - * dropped. - * notify_change() will delete those flags, make sure attr->ia_valid & ATTR_MODE - * is on, and remove the appropriate bits from attr->ia_mode (attr is a - * "struct iattr *"). -BlaisorBlade - */ struct hostfs_timespec { long long tv_sec; long long tv_nsec; -- 2.51.0 (that passes my usual tests, if you want you can apply it as is, or I can resend it as a real patch, or I can also put it into uml-next for later...) johannes
