Re: [RFC][PATCH 06/27] elevate write count during entire ncp_ioctl()

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

On Wed, Jun 07, 2006 at 05:10:18PM -0700, Dave Hansen wrote:
> 
> Some ioctls need write access, but others don't.  Make a helper
> function to decode when write access is needed, and take it.
> 
> Signed-off-by: Dave Hansen <haveblue@us.ibm.com>
> ---
> 
>  fs/nfsd/vfs.c             |    0 
>  lxc-dave/fs/ncpfs/ioctl.c |   54 +++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 53 insertions(+), 1 deletion(-)
> 
> diff -puN fs/namei.c~elevate-writers-file_permission-callers fs/namei.c
> diff -puN fs/open.c~elevate-writers-file_permission-callers fs/open.c
> diff -puN fs/exec.c~elevate-writers-file_permission-callers fs/exec.c
> diff -puN fs/ncpfs/ioctl.c~elevate-writers-file_permission-callers fs/ncpfs/ioctl.c
> --- lxc/fs/ncpfs/ioctl.c~elevate-writers-file_permission-callers	2006-06-07 16:53:15.000000000 -0700
> +++ lxc-dave/fs/ncpfs/ioctl.c	2006-06-07 16:53:15.000000000 -0700
> @@ -183,7 +183,7 @@ ncp_get_charsets(struct ncp_server* serv
>  }
>  #endif /* CONFIG_NCPFS_NLS */
>  
> -int ncp_ioctl(struct inode *inode, struct file *filp,
> +static int __ncp_ioctl(struct inode *inode, struct file *filp,
>  	      unsigned int cmd, unsigned long arg)
>  {
>  	struct ncp_server *server = NCP_SERVER(inode);
> @@ -654,3 +654,55 @@ outrel:			
>  /* #endif */
>  	return -EINVAL;
>  }
> +
> +static int ncp_ioctl_need_write(unsigned int cmd)
> +{
> +	switch (cmd) {
> +        case NCP_IOC_GET_FS_INFO:
> +        case NCP_IOC_GET_FS_INFO_V2:
> +        case NCP_IOC_NCPREQUEST:
> +        case NCP_IOC_SETDENTRYTTL:
> +        case NCP_IOC_SIGN_INIT:
> +        case NCP_IOC_LOCKUNLOCK:
> +        case NCP_IOC_SET_SIGN_WANTED:
> +		return 0;
> +        case NCP_IOC_GETOBJECTNAME:
> +        case NCP_IOC_SETOBJECTNAME:
> +        case NCP_IOC_GETPRIVATEDATA:
> +        case NCP_IOC_SETPRIVATEDATA:
> +        case NCP_IOC_SETCHARSETS:
> +        case NCP_IOC_GETCHARSETS:
> +        case NCP_IOC_CONN_LOGGED_IN:
> +        case NCP_IOC_GETDENTRYTTL:
> +        case NCP_IOC_GETMOUNTUID2:
> +        case NCP_IOC_SIGN_WANTED:
> +        case NCP_IOC_GETROOT:
> +        case NCP_IOC_SETROOT:
> +		return 0;

I'd assume one of those should be a return 1 :)

best,
Herbert

> +	default:
> +		/* unkown IOCTL command, assume write */
> +		WARN_ON();
> +	}
> +	return 1;
> +}
> +
> +int ncp_ioctl(struct inode *inode, struct file *filp,
> +	      unsigned int cmd, unsigned long arg)
> +{
> +	int ret;
> +
> +	if (ncp_ioctl_need_write(cmd)) {
> +		/*
> +		 * inside the ioctl(), any failures which
> +		 * are because of file_permission() are
> +		 * -EACCESS, so it seems consistent to keep
> +		 *  that here.
> +		 */
> +		if (mnt_want_write(filp->f_vfsmnt))
> +			return -EACCESS;
> +	}
> +	ret = __ncp_ioctl(inode, filp, cmd, arg);
> +	if (ncp_ioctl_need_write(cmd)
> +		mnt_drop_write(filp->->f_vfsmnt;
> +	return ret;
> +}
> diff -puN fs/nfsd/vfs.c~elevate-writers-file_permission-callers fs/nfsd/vfs.c
> _