xfs
[Top] [All Lists]

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

To: xfs@xxxxxxxxxxx
Subject: Re: [PATCH] use proper mnt_want_write calls for handle ioctls
From: Christoph Hellwig <hch@xxxxxx>
Date: Sat, 18 Oct 2008 14:37:39 +0200
In-reply-to: <20080929074234.GA23785@xxxxxx>
References: <20080814203952.GB18704@xxxxxx> <20080912084431.GA22641@xxxxxx> <20080929074234.GA23785@xxxxxx>
User-agent: Mutt/1.3.28i
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@xxxxxxxxxx>
> > > 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---

<Prev in Thread] Current Thread [Next in Thread>