Re: [PATCH] use proper mnt_want_write calls for handle ioctls

[Christoph Hellwig <hch@lst.de>]

Ping again.  As said before this could cause filesystem corruption with
the SGI tree, and it's always been correct in mainline there shouldn't
be any reason to delay it.

On Mon, Sep 29, 2008 at 09:42:34AM +0200, Christoph Hellwig wrote:
> ping^2?
> 
> On Fri, Sep 12, 2008 at 10:44:31AM +0200, Christoph Hellwig wrote:
> > ping?  Without this patch the CVS tree won't properly work with
> > read-only bind mounts.
> > 
> > On Thu, Aug 14, 2008 at 10:39:52PM +0200, Christoph Hellwig wrote:
> > > Since 2.6.26 all writes to filesystems need to be enclosed by a
> > > mnt_want_write / mnt_drop_write pair instead of checking for IS_RDONLY.
> > > 
> > > XFs was updated for this in mainline but the changes to xfs_ioctl.c
> > > were never megred back into the CVS tree.
> > > 
> > > The original commit introducing this was:
> > > 
> > > commit 42a74f206b914db13ee1f5ae932dcd91a77c8579
> > > Author: Dave Hansen <haveblue@us.ibm.com>
> > > Date:   Fri Feb 15 14:37:46 2008 -0800
> > > 
> > >     [PATCH] r/o bind mounts: elevate write count for ioctls()
> > > 
> > > Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_ioctl.c
> > > ===================================================================
> > > --- linux-2.6-xfs.orig/fs/xfs/linux-2.6/xfs_ioctl.c	2008-08-14 14:54:53.000000000 -0300
> > > +++ linux-2.6-xfs/fs/xfs/linux-2.6/xfs_ioctl.c	2008-08-14 15:15:49.000000000 -0300
> > > @@ -543,8 +543,6 @@ xfs_attrmulti_attr_set(
> > >  	char			*kbuf;
> > >  	int			error = EFAULT;
> > >  
> > > -	if (IS_RDONLY(inode))
> > > -		return -EROFS;
> > >  	if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
> > >  		return EPERM;
> > >  	if (len > XATTR_SIZE_MAX)
> > > @@ -570,8 +568,6 @@ xfs_attrmulti_attr_remove(
> > >  	char			*name,
> > >  	__uint32_t		flags)
> > >  {
> > > -	if (IS_RDONLY(inode))
> > > -		return -EROFS;
> > >  	if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
> > >  		return EPERM;
> > >  	return xfs_attr_remove(XFS_I(inode), name, flags);
> > > @@ -581,6 +577,7 @@ STATIC int
> > >  xfs_attrmulti_by_handle(
> > >  	xfs_mount_t		*mp,
> > >  	void			__user *arg,
> > > +	struct file		*parfilp,
> > >  	struct inode		*parinode)
> > >  {
> > >  	int			error;
> > > @@ -634,13 +631,21 @@ xfs_attrmulti_by_handle(
> > >  					&ops[i].am_length, ops[i].am_flags);
> > >  			break;
> > >  		case ATTR_OP_SET:
> > > +			ops[i].am_error = mnt_want_write(parfilp->f_path.mnt);
> > > +			if (ops[i].am_error)
> > > +				break;
> > >  			ops[i].am_error = xfs_attrmulti_attr_set(inode,
> > >  					attr_name, ops[i].am_attrvalue,
> > >  					ops[i].am_length, ops[i].am_flags);
> > > +			mnt_drop_write(parfilp->f_path.mnt);
> > >  			break;
> > >  		case ATTR_OP_REMOVE:
> > > +			ops[i].am_error = mnt_want_write(parfilp->f_path.mnt);
> > > +			if (ops[i].am_error)
> > > +				break;
> > >  			ops[i].am_error = xfs_attrmulti_attr_remove(inode,
> > >  					attr_name, ops[i].am_flags);
> > > +			mnt_drop_write(parfilp->f_path.mnt);
> > >  			break;
> > >  		default:
> > >  			ops[i].am_error = EINVAL;
> > > @@ -1431,7 +1436,7 @@ xfs_ioctl(
> > >  		return xfs_attrlist_by_handle(mp, arg, inode);
> > >  
> > >  	case XFS_IOC_ATTRMULTI_BY_HANDLE:
> > > -		return xfs_attrmulti_by_handle(mp, arg, inode);
> > > +		return xfs_attrmulti_by_handle(mp, arg, filp, inode);
> > >  
> > >  	case XFS_IOC_SWAPEXT: {
> > >  		error = xfs_swapext((struct xfs_swapext __user *)arg);
> > ---end quoted text---
> ---end quoted text---
---end quoted text---