public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] use proper mnt_want_write calls for handle ioctls
       [not found] <20080814203952.GB18704@lst.de>
@ 2008-09-12  8:44 ` Christoph Hellwig
  2008-09-29  7:42   ` Christoph Hellwig
  0 siblings, 1 reply; 4+ messages in thread
From: Christoph Hellwig @ 2008-09-12  8:44 UTC (permalink / raw)
  To: xfs

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---

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] use proper mnt_want_write calls for handle ioctls
  2008-09-12  8:44 ` [PATCH] use proper mnt_want_write calls for handle ioctls Christoph Hellwig
@ 2008-09-29  7:42   ` Christoph Hellwig
  2008-10-18 12:37     ` Christoph Hellwig
  0 siblings, 1 reply; 4+ messages in thread
From: Christoph Hellwig @ 2008-09-29  7:42 UTC (permalink / raw)
  To: xfs

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---

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] use proper mnt_want_write calls for handle ioctls
  2008-09-29  7:42   ` Christoph Hellwig
@ 2008-10-18 12:37     ` Christoph Hellwig
  2008-10-22  5:24       ` Donald Douwsma
  0 siblings, 1 reply; 4+ messages in thread
From: Christoph Hellwig @ 2008-10-18 12:37 UTC (permalink / raw)
  To: xfs

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---

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] use proper mnt_want_write calls for handle ioctls
  2008-10-18 12:37     ` Christoph Hellwig
@ 2008-10-22  5:24       ` Donald Douwsma
  0 siblings, 0 replies; 4+ messages in thread
From: Donald Douwsma @ 2008-10-22  5:24 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

Christoph Hellwig wrote:
> 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.

Done

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2008-10-22  5:22 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20080814203952.GB18704@lst.de>
2008-09-12  8:44 ` [PATCH] use proper mnt_want_write calls for handle ioctls Christoph Hellwig
2008-09-29  7:42   ` Christoph Hellwig
2008-10-18 12:37     ` Christoph Hellwig
2008-10-22  5:24       ` Donald Douwsma

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox