linux-kernel - Re: [PATCH 4/7] VFS: introduce dentry

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: <CAOQ4uxhiDNWjZXGhE31ZBPC_gUStETh4gyE8WxCRgiefiTCjCg@mail.gmail.com>
Date: Mon, 21 Jul 2025 12:20:14 +0200
From: Amir Goldstein <amir73il@...il.com>
To: NeilBrown <neil@...wn.name>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>, Alexander Viro <viro@...iv.linux.org.uk>, 
	Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>, linux-fsdevel@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/7] VFS: introduce dentry_lookup() and friends

On Mon, Jul 21, 2025 at 10:55 AM NeilBrown <neil@...wn.name> wrote:
>
> dentry_lookup() combines locking the directory and performing a lookup
> prior to a change to the directory.
> Abstracting this prepares for changing the locking requirements.
>
> dentry_lookup_noperm() does the same without needing a mnt_idmap and
> without checking permissions.  This is useful for internal filesystem
> management (e.g.  creating virtual files in response to events) and in
> other cases similar to lookup_noperm().
>
> dentry_lookup_hashed() also does no permissions checking and assumes
> that the hash of the name has already been stored in the qstr.

That's a very confusing choice of name because _hashed() (to me) sounds
like the opposite of d_unhashed() which is not at all the case.

> This is useful following filename_parentat().
>
> These are intended to be paired with done_dentry_lookup() which provides
> the inverse of putting the dentry and unlocking.
>
> Like lookup_one_qstr_excl(), dentry_lookup() returns -ENOENT if
> LOOKUP_CREATE was NOT given and the name cannot be found,, and returns
> -EEXIST if LOOKUP_EXCL WAS given and the name CAN be found.
>
> These functions replace all uses of lookup_one_qstr_excl() in namei.c
> except for those used for rename.
>
> Some of the variants should possibly be inlines in a header.
>
> Signed-off-by: NeilBrown <neil@...wn.name>
> ---
>  fs/namei.c            | 158 ++++++++++++++++++++++++++++++------------
>  include/linux/namei.h |   8 ++-
>  2 files changed, 119 insertions(+), 47 deletions(-)
>
> diff --git a/fs/namei.c b/fs/namei.c
> index 950a0d0d54da..f292df61565a 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -1714,17 +1714,98 @@ struct dentry *lookup_one_qstr_excl(const struct qstr *name,
>  }
>  EXPORT_SYMBOL(lookup_one_qstr_excl);
>
> +/**
> + * dentry_lookup_hashed - lookup and lock a name prior to dir ops
> + * @last: the name in the given directory
> + * @base: the directory in which the name is to be found
> + * @lookup_flags: %LOOKUP_xxx flags
> + *
> + * The name is looked up and necessary locks are taken so that
> + * the name can be created or removed.
> + * The "necessary locks" are currently the inode node lock on @base.
> + * The name @last is expected to already have the hash calculated.
> + * No permission checks are performed.
> + * Returns: the dentry, suitably locked, or an ERR_PTR().
> + */
> +struct dentry *dentry_lookup_hashed(struct qstr *last,
> +                                   struct dentry *base,
> +                                   unsigned int lookup_flags)
> +{
> +       struct dentry *dentry;
> +
> +       inode_lock_nested(base->d_inode, I_MUTEX_PARENT);
> +
> +       dentry = lookup_one_qstr_excl(last, base, lookup_flags);
> +       if (IS_ERR(dentry))
> +               inode_unlock(base->d_inode);
> +       return dentry;
> +}
> +EXPORT_SYMBOL(dentry_lookup_hashed);

Observation:

This part could be factored out of
__kern_path_locked()/kern_path_locked_negative()

If you do that in patch 2 while introducing done_dentry_lookup() then
it also makes
a lot of sense to balance the introduced done_dentry_lookup() with the
factored out
helper __dentry_lookup_locked() or whatever its name is.

Thanks,
Amir.