[PATCH 0/2] 2.6.29 queue

[PATCH 0/2] 2.6.29 queue

[Christoph Hellwig]

Two more updates for 2.6.29, one to fix a possible oops when we're out
of memory in xfs_log_mount and one to reject swapext on swapfiles.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 1/2] xfs: fix error handling in xfs_log_mount

[Christoph Hellwig]

[-- Attachment #1: xfs-log-mount-error-handling --]
[-- Type: text/plain, Size: 1410 bytes --]

We can't just call xfs_log_unmount_dealloc on any failure because the
ail thread which is torn down by xfs_log_unmount_dealloc might not
be initialized yet.


Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfs/fs/xfs/xfs_log.c
===================================================================
--- xfs.orig/fs/xfs/xfs_log.c	2009-02-10 19:16:18.761945594 +0100
+++ xfs/fs/xfs/xfs_log.c	2009-02-10 19:18:54.177068965 +0100
@@ -574,7 +574,7 @@ xfs_log_mount(
 	error = xfs_trans_ail_init(mp);
 	if (error) {
 		cmn_err(CE_WARN, "XFS: AIL initialisation failed: error %d", error);
-		goto error;
+		goto out_free_log;
 	}
 	mp->m_log->l_ailp = mp->m_ail;
 
@@ -594,20 +594,22 @@ xfs_log_mount(
 			mp->m_flags |= XFS_MOUNT_RDONLY;
 		if (error) {
 			cmn_err(CE_WARN, "XFS: log mount/recovery failed: error %d", error);
-			goto error;
+			goto out_destroy_ail;
 		}
 	}
 
 	/* Normal transactions can now occur */
 	mp->m_log->l_flags &= ~XLOG_ACTIVE_RECOVERY;
 
-	/* End mounting message in xfs_log_mount_finish */
 	return 0;
-error:
-	xfs_log_unmount_dealloc(mp);
+
+out_destroy_ail:
+	xfs_trans_ail_destroy(mp);
+out_free_log:
+	xlog_dealloc_log(mp->m_log);
 out:
 	return error;
-}	/* xfs_log_mount */
+}
 
 /*
  * Finish the recovery of the file system.  This is separate from

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 2/2] xfs: reject swapext ioctl on swapfiles

[Christoph Hellwig]

[-- Attachment #1: xfs-swapext-reject-swapfiles --]
[-- Type: text/plain, Size: 904 bytes --]

Swapfiles are magic - I/O is directly initialized by the VM without
involving the filesystem.  Swapping out extents underneath the VM thus
can cause severe problems.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfs/fs/xfs/xfs_dfrag.c
===================================================================
--- xfs.orig/fs/xfs/xfs_dfrag.c	2009-02-10 19:19:25.440068967 +0100
+++ xfs/fs/xfs/xfs_dfrag.c	2009-02-10 19:20:35.182944272 +0100
@@ -79,6 +79,12 @@ xfs_swapext(
 		goto out_put_target_file;
 	}
 
+	if (IS_SWAPFILE(file->f_path.dentry->d_inode) ||
+	    IS_SWAPFILE(target_file->f_path.dentry->d_inode)) {
+		error = XFS_ERROR(EINVAL);
+		goto out_put_target_file;
+	}
+
 	ip = XFS_I(file->f_path.dentry->d_inode);
 	tip = XFS_I(target_file->f_path.dentry->d_inode);
 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/2] xfs: fix error handling in xfs_log_mount

[Felix Blyakher]

Isn't it a reworked Lachlan's patch:

http://oss.sgi.com/archives/xfs/2009-02/msg00175.html

Otherwise, looks good.

Felix

On Feb 10, 2009, at 1:44 PM, Christoph Hellwig wrote:

> We can't just call xfs_log_unmount_dealloc on any failure because the
> ail thread which is torn down by xfs_log_unmount_dealloc might not
> be initialized yet.
>
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
>
> Index: xfs/fs/xfs/xfs_log.c
> ===================================================================
> --- xfs.orig/fs/xfs/xfs_log.c	2009-02-10 19:16:18.761945594 +0100
> +++ xfs/fs/xfs/xfs_log.c	2009-02-10 19:18:54.177068965 +0100
> @@ -574,7 +574,7 @@ xfs_log_mount(
> 	error = xfs_trans_ail_init(mp);
> 	if (error) {
> 		cmn_err(CE_WARN, "XFS: AIL initialisation failed: error %d", error);
> -		goto error;
> +		goto out_free_log;
> 	}
> 	mp->m_log->l_ailp = mp->m_ail;
>
> @@ -594,20 +594,22 @@ xfs_log_mount(
> 			mp->m_flags |= XFS_MOUNT_RDONLY;
> 		if (error) {
> 			cmn_err(CE_WARN, "XFS: log mount/recovery failed: error %d",  
> error);
> -			goto error;
> +			goto out_destroy_ail;
> 		}
> 	}
>
> 	/* Normal transactions can now occur */
> 	mp->m_log->l_flags &= ~XLOG_ACTIVE_RECOVERY;
>
> -	/* End mounting message in xfs_log_mount_finish */
> 	return 0;
> -error:
> -	xfs_log_unmount_dealloc(mp);
> +
> +out_destroy_ail:
> +	xfs_trans_ail_destroy(mp);
> +out_free_log:
> +	xlog_dealloc_log(mp->m_log);
> out:
> 	return error;
> -}	/* xfs_log_mount */
> +}
>
> /*
>  * Finish the recovery of the file system.  This is separate from
>
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/2] xfs: fix error handling in xfs_log_mount

[Christoph Hellwig]

On Tue, Feb 10, 2009 at 01:56:31PM -0600, Felix Blyakher wrote:
> Isn't it a reworked Lachlan's patch:
>
> http://oss.sgi.com/archives/xfs/2009-02/msg00175.html

The patch isn't related to that one at all, but it aims to fix the
same issue.  Should probably get a Reported-by: tag for him, if
this is indeed the thing the original patch tried to fix.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/2] xfs: reject swapext ioctl on swapfiles

[Felix Blyakher]

On Feb 10, 2009, at 1:44 PM, Christoph Hellwig wrote:

> Swapfiles are magic - I/O is directly initialized by the VM without
> involving the filesystem.  Swapping out extents underneath the VM thus
> can cause severe problems.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Felix Blyakher <felixb@sgi.com>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/2] xfs: fix error handling in xfs_log_mount

[Felix Blyakher]

On Feb 10, 2009, at 1:57 PM, Christoph Hellwig wrote:

> On Tue, Feb 10, 2009 at 01:56:31PM -0600, Felix Blyakher wrote:
>> Isn't it a reworked Lachlan's patch:
>>
>> http://oss.sgi.com/archives/xfs/2009-02/msg00175.html
>
> The patch isn't related to that one at all,

... from the implementation point of view, yes.

> but it aims to fix the
> same issue.

That's what I meant.

> Should probably get a Reported-by: tag for him,

Seems reasonable.

> if
> this is indeed the thing the original patch tried to fix.

That was my understanding. Let's Lachlan pitch in.

Felix

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/2] xfs: fix error handling in xfs_log_mount

[Lachlan McIlroy]

Felix Blyakher wrote:
> 
> On Feb 10, 2009, at 1:57 PM, Christoph Hellwig wrote:
> 
>> On Tue, Feb 10, 2009 at 01:56:31PM -0600, Felix Blyakher wrote:
>>> Isn't it a reworked Lachlan's patch:
>>>
>>> http://oss.sgi.com/archives/xfs/2009-02/msg00175.html
>>
>> The patch isn't related to that one at all,
> 
> ... from the implementation point of view, yes.
> 
>> but it aims to fix the
>> same issue.
> 
> That's what I meant.
> 
>> Should probably get a Reported-by: tag for him,
> 
> Seems reasonable.
> 
>> if
>> this is indeed the thing the original patch tried to fix.
> 
> That was my understanding. Let's Lachlan pitch in.
I'd prefer both fixes go in so we have maximum defence against
this problem happening again but I'm really not fussed.

> 
> Felix
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/2] xfs: fix error handling in xfs_log_mount

[Christoph Hellwig]

On Wed, Feb 11, 2009 at 10:24:37AM +1100, Lachlan McIlroy wrote:
> I'd prefer both fixes go in so we have maximum defence against
> this problem happening again but I'm really not fussed.

I disagree.  All these weird error checks for things that can't happen
just make the code big, bloated and unreadable.  And they make people
sloppy by not enforcing the same unwind order thus leading to more
bugs (as seen by the historic xfs mount path before we started unwinding
it)

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 0/2] 2.6.29 queue

[Christoph Hellwig]

On Tue, Feb 10, 2009 at 02:44:22PM -0500, Christoph Hellwig wrote:
> Two more updates for 2.6.29, one to fix a possible oops when we're out
> of memory in xfs_log_mount and one to reject swapext on swapfiles.

Thanks for the reviews!

The patches are now available to pull from:

		git://git.kernel.org/pub/scm/fs/xfs/xfs.git
	

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs