Re: [RFC][PATCH 22/27] sys_mknodat(): elevate write count for vfs_mknod/create()

[Herbert Poetzl <herbert@13thfloor.at>]

On Wed, Jun 07, 2006 at 05:10:35PM -0700, Dave Hansen wrote:
> This takes care of all of the direct callers of vfs_mknod().
> Since a few of these cases also handle normal file creation
> as well, this also covers some calls to vfs_create().
> 
> Signed-off-by: Dave Hansen <haveblue@us.ibm.com>
> ---
> 
>  ipc/mqueue.c                |    0 
>  lxc-dave/fs/namei.c         |   48 ++++++++++++++++++++++++++++----------------
>  lxc-dave/fs/nfsd/vfs.c      |    4 +++
>  lxc-dave/net/unix/af_unix.c |    4 +++
>  4 files changed, 39 insertions(+), 17 deletions(-)
> 
> diff -puN fs/namei.c~elevate-writers-vfs_mknod-try2 fs/namei.c
> --- lxc/fs/namei.c~elevate-writers-vfs_mknod-try2	2006-06-07 16:53:25.000000000 -0700
> +++ lxc-dave/fs/namei.c	2006-06-07 16:53:25.000000000 -0700
> @@ -1852,26 +1852,40 @@ asmlinkage long sys_mknodat(int dfd, con
>  
>  	if (!IS_POSIXACL(nd.dentry->d_inode))
>  		mode &= ~current->fs->umask;
> -	if (!IS_ERR(dentry)) {
> -		switch (mode & S_IFMT) {
> -		case 0: case S_IFREG:
> -			error = vfs_create(nd.dentry->d_inode,dentry,mode,&nd);
> -			break;
> -		case S_IFCHR: case S_IFBLK:
> -			error = vfs_mknod(nd.dentry->d_inode,dentry,mode,
> -					new_decode_dev(dev));
> -			break;
> -		case S_IFIFO: case S_IFSOCK:
> -			error = vfs_mknod(nd.dentry->d_inode,dentry,mode,0);
> +	if (IS_ERR(dentry))
> +		goto out_unlock;
> +	error = mnt_want_write(nd.mnt);
> +	if (error)
> +		goto out_dput;
> +	switch (mode & S_IFMT) {
> +	case 0: case S_IFREG:
> +		error = vfs_create(nd.dentry->d_inode,dentry,mode,&nd);
> +		break;
> +	case S_IFCHR: case S_IFBLK:

hmm, could you outline some cases where the following 
fails, but the previous (above) check succeeded?
(except for dubious locking issues)

> +		error = mnt_want_write(nd.mnt);
> +		if (error)
>  			break;
> -		case S_IFDIR:
> -			error = -EPERM;
> +		error = vfs_mknod(nd.dentry->d_inode,dentry,mode,
> +				new_decode_dev(dev));
> +		mnt_drop_write(nd.mnt);
> +		break;
> +	case S_IFIFO: case S_IFSOCK:

same here

> +		error = mnt_want_write(nd.mnt);
> +		if (error)
>  			break;
> -		default:
> -			error = -EINVAL;
> -		}
> -		dput(dentry);
> +		error = vfs_mknod(nd.dentry->d_inode,dentry,mode,0);
> +		mnt_drop_write(nd.mnt);
> +		break;
> +	case S_IFDIR:
> +		error = -EPERM;

this will give -EPERM on a r/w filesystem, but
-EROFS on r/o, is that consistant with the current
behaviour?

> +		break;
> +	default:
> +		error = -EINVAL;

same here for -EINVAL and -EROFS, maybe the 
mnt_want_write() wants to go into vfs_mknod()?

TIA,
Herbert

>  	}
> +	mnt_drop_write(nd.mnt);
> +out_dput:
> +	dput(dentry);
> +out_unlock:
>  	mutex_unlock(&nd.dentry->d_inode->i_mutex);
>  	path_release(&nd);
>  out:
> diff -puN fs/nfsd/vfs.c~elevate-writers-vfs_mknod-try2 fs/nfsd/vfs.c
> --- lxc/fs/nfsd/vfs.c~elevate-writers-vfs_mknod-try2	2006-06-07 16:53:25.000000000 -0700
> +++ lxc-dave/fs/nfsd/vfs.c	2006-06-07 16:53:25.000000000 -0700
> @@ -1153,6 +1153,9 @@ nfsd_create(struct svc_rqst *rqstp, stru
>  	/*
>  	 * Get the dir op function pointer.
>  	 */
> +	err = mnt_want_write(fhp->fh_export->ex_mnt);
> +	if (err)
> +		goto out_nfserr;
>  	err = nfserr_perm;
>  	switch (type) {
>  	case S_IFREG:
> @@ -1171,6 +1174,7 @@ nfsd_create(struct svc_rqst *rqstp, stru
>  	        printk("nfsd: bad file type %o in nfsd_create\n", type);
>  		err = -EINVAL;
>  	}
> +	mnt_drop_write(fhp->fh_export->ex_mnt);
>  	if (err < 0)
>  		goto out_nfserr;
>  
> diff -puN net/unix/af_unix.c~elevate-writers-vfs_mknod-try2 net/unix/af_unix.c
> --- lxc/net/unix/af_unix.c~elevate-writers-vfs_mknod-try2	2006-06-07 16:53:25.000000000 -0700
> +++ lxc-dave/net/unix/af_unix.c	2006-06-07 16:53:25.000000000 -0700
> @@ -789,7 +789,11 @@ static int unix_bind(struct socket *sock
>  		 */
>  		mode = S_IFSOCK |
>  		       (SOCK_INODE(sock)->i_mode & ~current->fs->umask);
> +		err = mnt_want_write(nd.mnt);
> +		if (err)
> +			goto out_mknod_dput;
>  		err = vfs_mknod(nd.dentry->d_inode, dentry, mode, 0);
> +		mnt_drop_write(nd.mnt);
>  		if (err)
>  			goto out_mknod_dput;
>  		mutex_unlock(&nd.dentry->d_inode->i_mutex);
> diff -puN ipc/mqueue.c~elevate-writers-vfs_mknod-try2 ipc/mqueue.c
> diff -L xattr.patch -puN /dev/null /dev/null
> _