Re: [PATCH 01/12] ovl: use is_subdir() for testing if one thing is a subdir of another

[Amir Goldstein <amir73il@xxxxxxxxx>]

On Wed, Jun 25, 2025 at 1:07 AM NeilBrown <neil@xxxxxxxxxx> wrote:
>
> Rather than using lock_rename(), use the more obvious is_subdir() for
> ensuring that neither upper nor workdir contain the other.
> Also be explicit in the comment that the two directories cannot be the
> same.
>
> Signed-off-by: NeilBrown <neil@xxxxxxxxxx>
> ---
>  fs/overlayfs/super.c | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index cf99b276fdfb..db046b0d6a68 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -438,18 +438,12 @@ static int ovl_lower_dir(const char *name, struct path *path,
>         return 0;
>  }
>
> -/* Workdir should not be subdir of upperdir and vice versa */
> +/* Workdir should not be subdir of upperdir and vice versa, and
> + * they should not be the same.
> + */
>  static bool ovl_workdir_ok(struct dentry *workdir, struct dentry *upperdir)
>  {
> -       bool ok = false;
> -
> -       if (workdir != upperdir) {
> -               struct dentry *trap = lock_rename(workdir, upperdir);
> -               if (!IS_ERR(trap))
> -                       unlock_rename(workdir, upperdir);
> -               ok = (trap == NULL);
> -       }
> -       return ok;
> +       return !is_subdir(workdir, upperdir) && !is_subdir(upperdir, workdir);

I am not at ease with this simplification to an unlocked test.
In the cover letter you wrote that this patch is not like the other patches.
Is this really necessary for your work?
If not, please leave it out.

Thanks,
Amir.