Re: [PATCH v7 3/3] ksmbd: fix racy issue from using ->d_parent and ->d_name

[Al Viro <viro@zeniv.linux.org.uk>]

On Wed, Sep 21, 2022 at 07:43:38AM +0900, Namjae Jeon wrote:


> -int ksmbd_vfs_kern_path(struct ksmbd_work *work, char *name,
> -			unsigned int flags, struct path *path, bool caseless)
> +int ksmbd_vfs_kern_path_locked(struct ksmbd_work *work, char *name,
> +			       unsigned int flags, struct path *path,
> +			       bool caseless)
>  {
>  	struct ksmbd_share_config *share_conf = work->tcon->share_conf;
>  	int err;
> +	struct path parent_path;
>  
> +	err = ksmbd_vfs_path_parent_lookup(share_conf, name, flags,
> +					   &parent_path);
>  	flags |= LOOKUP_BENEATH;
> -	err = vfs_path_lookup(share_conf->vfs_path.dentry,
> -			      share_conf->vfs_path.mnt,
> -			      name,
> -			      flags,
> -			      path);
> -	if (!err)
> -		return 0;
> +	if (!err) {
> +		err = vfs_path_lookup(share_conf->vfs_path.dentry,
> +				      share_conf->vfs_path.mnt,
> +				      name,
> +				      flags,
> +				      path);
> +		if (!err)
> +			goto lock_parent;
> +		path_put(&parent_path);

This is wrong.  You have already resolved the sucker to parent
+ last component.  Now you ask vfs_path_lookup() to
	* redo the same thing, hopefully arriving to the same
	  spot.
	* look the last component up in wherever it has arrived.
then you
	* lock the place you'd originally arrived at
	* check if the result of last lookup is its child (i.e.
it hadn't moved since we looked it up and lookup hopefully
arrived to the same spot for parent.

That's far too convoluted...