Re: [PATCH RFC] VFS: lock source directory for link to avoid rename race.

[Miklos Szeredi <miklos@szeredi.hu>]

On Fri, 16 Sept 2022 at 08:13, NeilBrown <neilb@suse.de> wrote:

> @@ -4554,44 +4590,83 @@ int do_linkat(int olddfd, struct filename *old, int newdfd,
>         if (flags & AT_SYMLINK_FOLLOW)
>                 how |= LOOKUP_FOLLOW;
>  retry:
> -       error = filename_lookup(olddfd, old, how, &old_path, NULL);
> +       err2 = 0;
> +       error = filename_parentat(olddfd, old, how, &old_path,
> +                                 &old_last, &old_type);
>         if (error)
>                 goto out_putnames;
> +       error = -EISDIR;
> +       if (old_type != LAST_NORM && !(flags & AT_EMPTY_PATH))
> +               goto out_putnames;
> +       error = filename_parentat(newdfd, new, (how & LOOKUP_REVAL), &new_path,
> +                                 &new_last, &new_type);
> +       if (error)
> +               goto out_putoldpath;
>
> -       new_dentry = filename_create(newdfd, new, &new_path,
> -                                       (how & LOOKUP_REVAL));
> -       error = PTR_ERR(new_dentry);
> -       if (IS_ERR(new_dentry))
> -               goto out_putpath;
> +       err2 = mnt_want_write(new_path.mnt);
>
>         error = -EXDEV;
>         if (old_path.mnt != new_path.mnt)
> -               goto out_dput;
> +               goto out_putnewpath;
> +       lock_link(new_path.dentry, old_path.dentry, flags);
> +
> +       new_dentry = __lookup_hash(&new_last, new_path.dentry, how & LOOKUP_REVAL);
> +       error = PTR_ERR(new_dentry);
> +       if (IS_ERR(new_dentry))
> +               goto out_unlock;
> +       error = -EEXIST;
> +       if (d_is_positive(new_dentry))
> +               goto out_dput_new;
> +       if (new_type != LAST_NORM)
> +               goto out_dput_new;
> +
> +       error = err2;
> +       if (error)
> +               goto out_dput_new;
> +
> +       if (flags & AT_EMPTY_PATH)
> +               old_dentry = dget(old_path.dentry);
> +       else
> +               old_dentry = __lookup_hash(&old_last, old_path.dentry, how);

This will break AT_SYMLINK_FOLLOW.

And yes, we can add all the lookup logic to do_linkat() at which point
it will about 10x more complex than it was.

Thanks,
Miklos