Re: [PATCH 08/13] vfs: make vfs_mknod break delegations on parent directory

On Mon 13-10-25 10:48:06, Jeff Layton wrote:
> In order to add directory delegation support, we need to break
> delegations on the parent whenever there is going to be a change in the
> directory.
> 
> Add a new delegated_inode return pointer to vfs_mknod() and have the
> appropriate callers wait when there is an outstanding delegation. All
> other callers just set the pointer to NULL.
> 
> Signed-off-by: Jeff Layton <[email protected]>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <[email protected]>

								Honza

> ---
>  drivers/base/devtmpfs.c  |  2 +-
>  fs/ecryptfs/inode.c      |  2 +-
>  fs/init.c                |  2 +-
>  fs/namei.c               | 25 +++++++++++++++++--------
>  fs/nfsd/vfs.c            |  2 +-
>  fs/overlayfs/overlayfs.h |  2 +-
>  include/linux/fs.h       |  4 ++--
>  net/unix/af_unix.c       |  2 +-
>  8 files changed, 25 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/base/devtmpfs.c b/drivers/base/devtmpfs.c
> index 104025104ef75381984fd94dfbd50feeaa8cdd22..2f576ecf18324f767cd5ac6cbd28adbf9f46b958 100644
> --- a/drivers/base/devtmpfs.c
> +++ b/drivers/base/devtmpfs.c
> @@ -231,7 +231,7 @@ static int handle_create(const char *nodename, umode_t mode, kuid_t uid,
>  		return PTR_ERR(dentry);
>  
>  	err = vfs_mknod(&nop_mnt_idmap, d_inode(path.dentry), dentry, mode,
> -			dev->devt);
> +			dev->devt, NULL);
>  	if (!err) {
>  		struct iattr newattrs;
>  
> diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
> index 88631291b32535f623a3fbe4ea9b6ed48a306ca0..acef6d921167268d4590c688894d4522016db0dd 100644
> --- a/fs/ecryptfs/inode.c
> +++ b/fs/ecryptfs/inode.c
> @@ -565,7 +565,7 @@ ecryptfs_mknod(struct mnt_idmap *idmap, struct inode *dir,
>  	rc = lock_parent(dentry, &lower_dentry, &lower_dir);
>  	if (!rc)
>  		rc = vfs_mknod(&nop_mnt_idmap, lower_dir,
> -			       lower_dentry, mode, dev);
> +			       lower_dentry, mode, dev, NULL);
>  	if (rc || d_really_is_negative(lower_dentry))
>  		goto out;
>  	rc = ecryptfs_interpose(lower_dentry, dentry, dir->i_sb);
> diff --git a/fs/init.c b/fs/init.c
> index 895f8a09a71acfd03e11164e3b441a7d4e2de146..4f02260dd65b0dfcbfbf5812d2ec6a33444a3b1f 100644
> --- a/fs/init.c
> +++ b/fs/init.c
> @@ -157,7 +157,7 @@ int __init init_mknod(const char *filename, umode_t mode, unsigned int dev)
>  	error = security_path_mknod(&path, dentry, mode, dev);
>  	if (!error)
>  		error = vfs_mknod(mnt_idmap(path.mnt), path.dentry->d_inode,
> -				  dentry, mode, new_decode_dev(dev));
> +				  dentry, mode, new_decode_dev(dev), NULL);
>  	end_creating_path(&path, dentry);
>  	return error;
>  }
> diff --git a/fs/namei.c b/fs/namei.c
> index 1427c53e13978e70adefdc572b71247536985430..2e1e3f0068a28271e07aa0fa0c7e0b04582400fe 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -4302,13 +4302,15 @@ inline struct dentry *start_creating_user_path(
>  }
>  EXPORT_SYMBOL(start_creating_user_path);
>  
> +
>  /**
>   * vfs_mknod - create device node or file
> - * @idmap:	idmap of the mount the inode was found from
> - * @dir:	inode of the parent directory
> - * @dentry:	dentry of the child device node
> - * @mode:	mode of the child device node
> - * @dev:	device number of device to create
> + * @idmap:		idmap of the mount the inode was found from
> + * @dir:		inode of the parent directory
> + * @dentry:		dentry of the child device node
> + * @mode:		mode of the child device node
> + * @dev:		device number of device to create
> + * @delegated_inode:	returns parent inode, if the inode is delegated.
>   *
>   * Create a device node or file.
>   *
> @@ -4319,7 +4321,8 @@ EXPORT_SYMBOL(start_creating_user_path);
>   * raw inode simply pass @nop_mnt_idmap.
>   */
>  int vfs_mknod(struct mnt_idmap *idmap, struct inode *dir,
> -	      struct dentry *dentry, umode_t mode, dev_t dev)
> +	      struct dentry *dentry, umode_t mode, dev_t dev,
> +	      struct inode **delegated_inode)
>  {
>  	bool is_whiteout = S_ISCHR(mode) && dev == WHITEOUT_DEV;
>  	int error = may_create(idmap, dir, dentry);
> @@ -4343,6 +4346,10 @@ int vfs_mknod(struct mnt_idmap *idmap, struct inode *dir,
>  	if (error)
>  		return error;
>  
> +	error = try_break_deleg(dir, delegated_inode);
> +	if (error)
> +		return error;
> +
>  	error = dir->i_op->mknod(idmap, dir, dentry, mode, dev);
>  	if (!error)
>  		fsnotify_create(dir, dentry);
> @@ -4402,11 +4409,13 @@ static int do_mknodat(int dfd, struct filename *name, umode_t mode,
>  			break;
>  		case S_IFCHR: case S_IFBLK:
>  			error = vfs_mknod(idmap, path.dentry->d_inode,
> -					  dentry, mode, new_decode_dev(dev));
> +					  dentry, mode, new_decode_dev(dev),
> +					  &delegated_inode);
>  			break;
>  		case S_IFIFO: case S_IFSOCK:
>  			error = vfs_mknod(idmap, path.dentry->d_inode,
> -					  dentry, mode, 0);
> +					  dentry, mode, 0,
> +					  &delegated_inode);
>  			break;
>  	}
>  out2:
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 7d8cd2595f197be9741ee6320d43ed6651896647..858485c76b6524e965b7cbc92f67c1a4eb19e34e 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -1660,7 +1660,7 @@ nfsd_create_locked(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  	case S_IFIFO:
>  	case S_IFSOCK:
>  		host_err = vfs_mknod(&nop_mnt_idmap, dirp, dchild,
> -				     iap->ia_mode, rdev);
> +				     iap->ia_mode, rdev, NULL);
>  		break;
>  	default:
>  		printk(KERN_WARNING "nfsd: bad file type %o in nfsd_create\n",
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index d215d7349489686b66bb66e939b27046f7d836f6..8b8c99e9e1a518c365cfff952d391887ec18d453 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -257,7 +257,7 @@ static inline int ovl_do_mknod(struct ovl_fs *ofs,
>  			       struct inode *dir, struct dentry *dentry,
>  			       umode_t mode, dev_t dev)
>  {
> -	int err = vfs_mknod(ovl_upper_mnt_idmap(ofs), dir, dentry, mode, dev);
> +	int err = vfs_mknod(ovl_upper_mnt_idmap(ofs), dir, dentry, mode, dev, NULL);
>  
>  	pr_debug("mknod(%pd2, 0%o, 0%o) = %i\n", dentry, mode, dev, err);
>  	return err;
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index d8bdaf7c87502ff17775602f5391d375738b4ed8..4ad49b39441b2c9088fd01a7e0e46a6511c26d2e 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2115,7 +2115,7 @@ int vfs_create(struct mnt_idmap *, struct inode *,
>  struct dentry *vfs_mkdir(struct mnt_idmap *, struct inode *,
>  			 struct dentry *, umode_t, struct inode **);
>  int vfs_mknod(struct mnt_idmap *, struct inode *, struct dentry *,
> -              umode_t, dev_t);
> +	      umode_t, dev_t, struct inode **);
>  int vfs_symlink(struct mnt_idmap *, struct inode *,
>  		struct dentry *, const char *);
>  int vfs_link(struct dentry *, struct mnt_idmap *, struct inode *,
> @@ -2151,7 +2151,7 @@ static inline int vfs_whiteout(struct mnt_idmap *idmap,
>  			       struct inode *dir, struct dentry *dentry)
>  {
>  	return vfs_mknod(idmap, dir, dentry, S_IFCHR | WHITEOUT_MODE,
> -			 WHITEOUT_DEV);
> +			 WHITEOUT_DEV, NULL);
>  }
>  
>  struct file *kernel_tmpfile_open(struct mnt_idmap *idmap,
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index 768098dec2310008632558ae928703b37c3cc8ef..db1fd8d6a84c2c7c0d45b43d9c5a936b3d491b7b 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -1399,7 +1399,7 @@ static int unix_bind_bsd(struct sock *sk, struct sockaddr_un *sunaddr,
>  	idmap = mnt_idmap(parent.mnt);
>  	err = security_path_mknod(&parent, dentry, mode, 0);
>  	if (!err)
> -		err = vfs_mknod(idmap, d_inode(parent.dentry), dentry, mode, 0);
> +		err = vfs_mknod(idmap, d_inode(parent.dentry), dentry, mode, 0, NULL);
>  	if (err)
>  		goto out_path;
>  	err = mutex_lock_interruptible(&u->bindlock);
> 
> -- 
> 2.51.0
> 
-- 
Jan Kara <[email protected]>
SUSE Labs, CR