On Wed, Sep 20, 2023 at 8:51 PM Bernd Schubert <bschubert@xxxxxxx> wrote:
>
> From: Miklos Szeredi <miklos@xxxxxxxxxx>
>
> atomic_open() will do an open-by-name or create-and-open
> depending on the flags.
>
> If file was created, then the old positive dentry is obviously
> stale, so it will be invalidated and a new one will be allocated.
>
> If not created, then check whether it's the same inode (same as in
> ->d_revalidate()) and if not, invalidate & allocate new dentry.
>
> Co-developed-by: Bernd Schubert <bschubert@xxxxxxx>
> Signed-off-by: Miklos Szeredi <miklos@xxxxxxxxxx>
> Signed-off-by: Bernd Schubert <bschubert@xxxxxxx>
> Cc: Christian Brauner <brauner@xxxxxxxxxx>
> Cc: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
> Cc: Dharmendra Singh <dsingh@xxxxxxx>
> Cc: linux-fsdevel@xxxxxxxxxxxxxxx
> ---
> fs/namei.c | 25 ++++++++++++++++++++-----
> include/linux/namei.h | 7 +++++++
> 2 files changed, 27 insertions(+), 5 deletions(-)
>
> diff --git a/fs/namei.c b/fs/namei.c
> index e56ff39a79bc..f01b278ac0ef 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -858,7 +858,7 @@ static inline int d_revalidate(struct dentry *dentry, unsigned int flags)
> if (unlikely(dentry->d_flags & DCACHE_OP_REVALIDATE))
> return dentry->d_op->d_revalidate(dentry, flags);
> else
> - return 1;
> + return D_REVALIDATE_VALID;
> }
>
> /**
> @@ -1611,10 +1611,11 @@ struct dentry *lookup_one_qstr_excl(const struct qstr *name,
> }
> EXPORT_SYMBOL(lookup_one_qstr_excl);
>
> -static struct dentry *lookup_fast(struct nameidata *nd)
> +static struct dentry *lookup_fast(struct nameidata *nd, bool *atomic_revalidate)
> {
> struct dentry *dentry, *parent = nd->path.dentry;
> int status = 1;
> + *atomic_revalidate = false;
>
> /*
> * Rename seqlock is not required here because in the off chance
> @@ -1656,6 +1657,10 @@ static struct dentry *lookup_fast(struct nameidata *nd)
> dput(dentry);
> return ERR_PTR(status);
> }
> +
> + if (status == D_REVALIDATE_ATOMIC)
> + *atomic_revalidate = true;
> +
> return dentry;
> }
>
> @@ -1981,6 +1986,7 @@ static const char *handle_dots(struct nameidata *nd, int type)
> static const char *walk_component(struct nameidata *nd, int flags)
> {
> struct dentry *dentry;
> + bool atomic_revalidate;
> /*
> * "." and ".." are special - ".." especially so because it has
> * to be able to know about the current root directory and
> @@ -1991,7 +1997,7 @@ static const char *walk_component(struct nameidata *nd, int flags)
> put_link(nd);
> return handle_dots(nd, nd->last_type);
> }
> - dentry = lookup_fast(nd);
> + dentry = lookup_fast(nd, &atomic_revalidate);
> if (IS_ERR(dentry))
> return ERR_CAST(dentry);
> if (unlikely(!dentry)) {
> @@ -1999,6 +2005,9 @@ static const char *walk_component(struct nameidata *nd, int flags)
> if (IS_ERR(dentry))
> return ERR_CAST(dentry);
> }
> +
> + WARN_ON(atomic_revalidate);
> +
WARN_ON_ONCE should be enough in most cases
to signal bad code without spamming the log.
Is there an error path that can be taken in this case?
Thanks,
Amir.
> if (!(flags & WALK_MORE) && nd->depth)
> put_link(nd);
> return step_into(nd, flags, dentry);
> @@ -3430,7 +3439,7 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
> dput(dentry);
> dentry = NULL;
> }
> - if (dentry->d_inode) {
> + if (dentry->d_inode && error != D_REVALIDATE_ATOMIC) {
> /* Cached positive dentry: will open in f_op->open */
> return dentry;
> }
> @@ -3523,12 +3532,18 @@ static const char *open_last_lookups(struct nameidata *nd,
> }
>
> if (!(open_flag & O_CREAT)) {
> + bool atomic_revalidate;
> +
> if (nd->last.name[nd->last.len])
> nd->flags |= LOOKUP_FOLLOW | LOOKUP_DIRECTORY;
> /* we _can_ be in RCU mode here */
> - dentry = lookup_fast(nd);
> + dentry = lookup_fast(nd, &atomic_revalidate);
> if (IS_ERR(dentry))
> return ERR_CAST(dentry);
> + if (dentry && unlikely(atomic_revalidate)) {
> + dput(dentry);
> + dentry = NULL;
> + }
> if (likely(dentry))
> goto finish_lookup;
>
> diff --git a/include/linux/namei.h b/include/linux/namei.h
> index 1463cbda4888..a70e87d2b2a9 100644
> --- a/include/linux/namei.h
> +++ b/include/linux/namei.h
> @@ -47,6 +47,13 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT};
> /* LOOKUP_* flags which do scope-related checks based on the dirfd. */
> #define LOOKUP_IS_SCOPED (LOOKUP_BENEATH | LOOKUP_IN_ROOT)
>
> +/* ->d_revalidate return codes */
> +enum {
> + D_REVALIDATE_INVALID = 0, /* invalid dentry */
> + D_REVALIDATE_VALID = 1, /* valid dentry */
> + D_REVALIDATE_ATOMIC = 2, /* atomic_open will revalidate */
> +};
> +
> extern int path_pts(struct path *path);
>
> extern int user_path_at_empty(int, const char __user *, unsigned, struct path *, int *empty);
> --
> 2.39.2
>
