linux-kernel - Re: [PATCH 07/19] VFS: repack LOOKUP

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: <20250206-wirren-ausfiel-99acf5b0ace8@brauner>
Date: Thu, 6 Feb 2025 13:44:13 +0100
From: Christian Brauner <brauner@...nel.org>
To: NeilBrown <neilb@...e.de>
Cc: Alexander Viro <viro@...iv.linux.org.uk>, Jan Kara <jack@...e.cz>, 
	Linus Torvalds <torvalds@...ux-foundation.org>, Jeff Layton <jlayton@...nel.org>, 
	Dave Chinner <david@...morbit.com>, linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 07/19] VFS: repack LOOKUP_ bit flags.

On Thu, Feb 06, 2025 at 04:42:44PM +1100, NeilBrown wrote:
> The LOOKUP_ bits are not in order, which can make it awkward when adding
> new bits.  Two bits have recently been added to the end which makes them
> look like "scoping flags", but in fact they aren't.
> 
> Also LOOKUP_PARENT is described as "internal use only" but is used in
> fs/nfs/
> 
> This patch:
>  - Moves these three flags into the "pathwalk mode" section
>  - changes all bits to use the BIT(n) macro
>  - Allocates bits in order leaving gaps between the sections,
>    and documents those gaps.
> 
> Signed-off-by: NeilBrown <neilb@...e.de>
> ---

This is also a worthwhile cleanup independent of the rest of the series.
But you've added LOOKUP_INTENT_FLAGS prior to packing the flags. Imho,
this patch should've gone before the addition of LOOKUP_INTENT_FLAGS.

And btw, what does this series apply to?
Doesn't apply to next-20250206 nor to current mainline.
I get the usual

Patch failed at 0012 VFS: enhance d_splice_alias to accommodate shared-lock updates
error: sha1 information is lacking or useless (fs/dcache.c).
error: could not build fake ancestor

when trying to look at this locally.

>  include/linux/namei.h | 46 +++++++++++++++++++++----------------------
>  1 file changed, 23 insertions(+), 23 deletions(-)
> 
> diff --git a/include/linux/namei.h b/include/linux/namei.h
> index 839a64d07f8c..0d81e571a159 100644
> --- a/include/linux/namei.h
> +++ b/include/linux/namei.h
> @@ -18,38 +18,38 @@ enum { MAX_NESTED_LINKS = 8 };
>  enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT};
>  
>  /* pathwalk mode */
> -#define LOOKUP_FOLLOW		0x0001	/* follow links at the end */
> -#define LOOKUP_DIRECTORY	0x0002	/* require a directory */
> -#define LOOKUP_AUTOMOUNT	0x0004  /* force terminal automount */
> -#define LOOKUP_EMPTY		0x4000	/* accept empty path [user_... only] */
> -#define LOOKUP_DOWN		0x8000	/* follow mounts in the starting point */
> -#define LOOKUP_MOUNTPOINT	0x0080	/* follow mounts in the end */
> -
> -#define LOOKUP_REVAL		0x0020	/* tell ->d_revalidate() to trust no cache */
> -#define LOOKUP_RCU		0x0040	/* RCU pathwalk mode; semi-internal */
> +#define LOOKUP_FOLLOW		BIT(0)	/* follow links at the end */
> +#define LOOKUP_DIRECTORY	BIT(1)	/* require a directory */
> +#define LOOKUP_AUTOMOUNT	BIT(2)  /* force terminal automount */
> +#define LOOKUP_EMPTY		BIT(3)	/* accept empty path [user_... only] */
> +#define LOOKUP_LINKAT_EMPTY	BIT(4) /* Linkat request with empty path. */
> +#define LOOKUP_DOWN		BIT(5)	/* follow mounts in the starting point */
> +#define LOOKUP_MOUNTPOINT	BIT(6)	/* follow mounts in the end */
> +#define LOOKUP_REVAL		BIT(7)	/* tell ->d_revalidate() to trust no cache */
> +#define LOOKUP_RCU		BIT(8)	/* RCU pathwalk mode; semi-internal */
> +#define LOOKUP_CACHED		BIT(9) /* Only do cached lookup */
> +#define LOOKUP_PARENT		BIT(10)	/* Looking up final parent in path */
> +/* 5 spare bits for pathwalk */
>  
>  /* These tell filesystem methods that we are dealing with the final component... */
> -#define LOOKUP_OPEN		0x0100	/* ... in open */
> -#define LOOKUP_CREATE		0x0200	/* ... in object creation */
> -#define LOOKUP_EXCL		0x0400	/* ... in target must not exist */
> -#define LOOKUP_RENAME_TARGET	0x0800	/* ... in destination of rename() */
> +#define LOOKUP_OPEN		BIT(16)	/* ... in open */
> +#define LOOKUP_CREATE		BIT(17)	/* ... in object creation */
> +#define LOOKUP_EXCL		BIT(18)	/* ... in target must not exist */
> +#define LOOKUP_RENAME_TARGET	BIT(19)	/* ... in destination of rename() */
>  
>  #define LOOKUP_INTENT_FLAGS	(LOOKUP_OPEN | LOOKUP_CREATE | LOOKUP_EXCL |	\
>  				 LOOKUP_RENAME_TARGET)
> -
> -/* internal use only */
> -#define LOOKUP_PARENT		0x0010
> +/* 4 spare bits for intent */
>  
>  /* Scoping flags for lookup. */
> -#define LOOKUP_NO_SYMLINKS	0x010000 /* No symlink crossing. */
> -#define LOOKUP_NO_MAGICLINKS	0x020000 /* No nd_jump_link() crossing. */
> -#define LOOKUP_NO_XDEV		0x040000 /* No mountpoint crossing. */
> -#define LOOKUP_BENEATH		0x080000 /* No escaping from starting point. */
> -#define LOOKUP_IN_ROOT		0x100000 /* Treat dirfd as fs root. */
> -#define LOOKUP_CACHED		0x200000 /* Only do cached lookup */
> -#define LOOKUP_LINKAT_EMPTY	0x400000 /* Linkat request with empty path. */
> +#define LOOKUP_NO_SYMLINKS	BIT(24) /* No symlink crossing. */
> +#define LOOKUP_NO_MAGICLINKS	BIT(25) /* No nd_jump_link() crossing. */
> +#define LOOKUP_NO_XDEV		BIT(26) /* No mountpoint crossing. */
> +#define LOOKUP_BENEATH		BIT(27) /* No escaping from starting point. */
> +#define LOOKUP_IN_ROOT		BIT(28) /* Treat dirfd as fs root. */
>  /* LOOKUP_* flags which do scope-related checks based on the dirfd. */
>  #define LOOKUP_IS_SCOPED (LOOKUP_BENEATH | LOOKUP_IN_ROOT)
> +/* 3 spare bits for scoping */
>  
>  extern int path_pts(struct path *path);
>  
> -- 
> 2.47.1
>