rpms/kernel/F-12 xfs_swap_extents-needs-to-handle-dynamic-fork-offsets.patch, 1.1, 1.2 kernel.spec, 1.2007, 1.2008

[Kyle McMartin]

Author: kyle

Update of /cvs/pkgs/rpms/kernel/F-12
In directory cvs1.fedora.phx.redhat.com:/tmp/cvs-serv6354

Modified Files:
	kernel.spec 
Added Files:
	xfs_swap_extents-needs-to-handle-dynamic-fork-offsets.patch 
Log Message:
* Sun Feb 07 2010 Kyle McMartin <kyle at redhat.com> 2.6.32.8-48.rc2
- xfs: xfs_swap_extents needs to handle dynamic fork offsets (rhbz#510823)
  from sandeen.


xfs_swap_extents-needs-to-handle-dynamic-fork-offsets.patch:
 xfs_dfrag.c |  107 ++++++++++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 90 insertions(+), 17 deletions(-)

Index: xfs_swap_extents-needs-to-handle-dynamic-fork-offsets.patch
===================================================================
RCS file: xfs_swap_extents-needs-to-handle-dynamic-fork-offsets.patch
diff -N xfs_swap_extents-needs-to-handle-dynamic-fork-offsets.patch
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ xfs_swap_extents-needs-to-handle-dynamic-fork-offsets.patch	7 Feb 2010 22:05:37 -0000	1.2
@@ -0,0 +1,181 @@
+From f05e8a7a55b38cfd53510333d8d315f47d3ece18 Mon Sep 17 00:00:00 2001
+From: Dave Chinner <david at fromorbit.com>
+Date: Thu, 14 Jan 2010 01:33:54 +0000
+Subject: xfs: xfs_swap_extents needs to handle dynamic fork offsets
+
+When swapping extents, we can corrupt inodes by swapping data forks
+that are in incompatible formats.  This is caused by the two indoes
+having different fork offsets due to the presence of an attribute
+fork on an attr2 filesystem.  xfs_fsr tries to be smart about
+setting the fork offset, but the trick it plays only works on attr1
+(old fixed format attribute fork) filesystems.
+
+Changing the way xfs_fsr sets up the attribute fork will prevent
+this situation from ever occurring, so in the kernel code we can get
+by with a preventative fix - check that the data fork in the
+defragmented inode is in a format valid for the inode it is being
+swapped into.  This will lead to files that will silently and
+potentially repeatedly fail defragmentation, so issue a warning to
+the log when this particular failure occurs to let us know that
+xfs_fsr needs updating/fixing.
+
+To help identify how to improve xfs_fsr to avoid this issue, add
+trace points for the inodes being swapped so that we can determine
+why the swap was rejected and to confirm that the code is making the
+right decisions and modifications when swapping forks.
+
+A further complication is even when the swap is allowed to proceed
+when the fork offset is different between the two inodes then value
+for the maximum number of extents the data fork can hold can be
+wrong. Make sure these are also set correctly after the swap occurs.
+
+Signed-off-by: Dave Chinner <david at fromorbit.com>
+Reviewed-by: Christoph Hellwig <hch at lst.de>
+Signed-off-by: Alex Elder <aelder at sgi.com>
+---
+ fs/xfs/xfs_dfrag.c |  106 ++++++++++++++++++++++++++++++++++++++++++++--------
+ 1 files changed, 90 insertions(+), 16 deletions(-)
+
+diff --git a/fs/xfs/xfs_dfrag.c b/fs/xfs/xfs_dfrag.c
+index 7465f9e..0172d94 100644
+--- a/fs/xfs/xfs_dfrag.c
++++ b/fs/xfs/xfs_dfrag.c
+@@ -113,10 +113,82 @@ xfs_swapext(
+ 	return error;
+ }
+ 
++/*
++ * We need to check that the format of the data fork in the temporary inode is
++ * valid for the target inode before doing the swap. This is not a problem with
++ * attr1 because of the fixed fork offset, but attr2 has a dynamically sized
++ * data fork depending on the space the attribute fork is taking so we can get
++ * invalid formats on the target inode.
++ *
++ * E.g. target has space for 7 extents in extent format, temp inode only has
++ * space for 6.  If we defragment down to 7 extents, then the tmp format is a
++ * btree, but when swapped it needs to be in extent format. Hence we can't just
++ * blindly swap data forks on attr2 filesystems.
++ *
++ * Note that we check the swap in both directions so that we don't end up with
++ * a corrupt temporary inode, either.
++ *
++ * Note that fixing the way xfs_fsr sets up the attribute fork in the source
++ * inode will prevent this situation from occurring, so all we do here is
++ * reject and log the attempt. basically we are putting the responsibility on
++ * userspace to get this right.
++ */
++static int
++xfs_swap_extents_check_format(
++	xfs_inode_t	*ip,	/* target inode */
++	xfs_inode_t	*tip)	/* tmp inode */
++{
++
++	/* Should never get a local format */
++	if (ip->i_d.di_format == XFS_DINODE_FMT_LOCAL ||
++	    tip->i_d.di_format == XFS_DINODE_FMT_LOCAL)
++		return EINVAL;
++
++	/*
++	 * if the target inode has less extents that then temporary inode then
++	 * why did userspace call us?
++	 */
++	if (ip->i_d.di_nextents < tip->i_d.di_nextents)
++		return EINVAL;
++
++	/*
++	 * if the target inode is in extent form and the temp inode is in btree
++	 * form then we will end up with the target inode in the wrong format
++	 * as we already know there are less extents in the temp inode.
++	 */
++	if (ip->i_d.di_format == XFS_DINODE_FMT_EXTENTS &&
++	    tip->i_d.di_format == XFS_DINODE_FMT_BTREE)
++		return EINVAL;
++
++	/* Check temp in extent form to max in target */
++	if (tip->i_d.di_format == XFS_DINODE_FMT_EXTENTS &&
++	    XFS_IFORK_NEXTENTS(tip, XFS_DATA_FORK) > ip->i_df.if_ext_max)
++		return EINVAL;
++
++	/* Check target in extent form to max in temp */
++	if (ip->i_d.di_format == XFS_DINODE_FMT_EXTENTS &&
++	    XFS_IFORK_NEXTENTS(ip, XFS_DATA_FORK) > tip->i_df.if_ext_max)
++		return EINVAL;
++
++	/* Check root block of temp in btree form to max in target */
++	if (tip->i_d.di_format == XFS_DINODE_FMT_BTREE &&
++	    XFS_IFORK_BOFF(ip) &&
++	    tip->i_df.if_broot_bytes > XFS_IFORK_BOFF(ip))
++		return EINVAL;
++
++	/* Check root block of target in btree form to max in temp */
++	if (ip->i_d.di_format == XFS_DINODE_FMT_BTREE &&
++	    XFS_IFORK_BOFF(tip) &&
++	    ip->i_df.if_broot_bytes > XFS_IFORK_BOFF(tip))
++		return EINVAL;
++
++	return 0;
++}
++
+ int
+ xfs_swap_extents(
+-	xfs_inode_t	*ip,
+-	xfs_inode_t	*tip,
++	xfs_inode_t	*ip,	/* target inode */
++	xfs_inode_t	*tip,	/* tmp inode */
+ 	xfs_swapext_t	*sxp)
+ {
+ 	xfs_mount_t	*mp;
+@@ -160,13 +232,6 @@ xfs_swap_extents(
+ 		goto out_unlock;
+ 	}
+ 
+-	/* Should never get a local format */
+-	if (ip->i_d.di_format == XFS_DINODE_FMT_LOCAL ||
+-	    tip->i_d.di_format == XFS_DINODE_FMT_LOCAL) {
+-		error = XFS_ERROR(EINVAL);
+-		goto out_unlock;
+-	}
+-
+ 	if (VN_CACHED(VFS_I(tip)) != 0) {
+ 		xfs_inval_cached_trace(tip, 0, -1, 0, -1);
+ 		error = xfs_flushinval_pages(tip, 0, -1,
+@@ -189,13 +254,12 @@ xfs_swap_extents(
+ 		goto out_unlock;
+ 	}
+ 
+-	/*
+-	 * If the target has extended attributes, the tmp file
+-	 * must also in order to ensure the correct data fork
+-	 * format.
+-	 */
+-	if ( XFS_IFORK_Q(ip) != XFS_IFORK_Q(tip) ) {
+-		error = XFS_ERROR(EINVAL);
++	/* check inode formats now that data is flushed */
++	error = xfs_swap_extents_check_format(ip, tip);
++	if (error) {
++		xfs_fs_cmn_err(CE_NOTE, mp,
++		    "%s: inode 0x%llx format is incompatible for exchanging.",
++				__FILE__, ip->i_ino);
+ 		goto out_unlock;
+ 	}
+ 
+@@ -276,6 +340,16 @@ xfs_swap_extents(
+ 	*tifp = *tempifp;	/* struct copy */
+ 
+ 	/*
++	 * Fix the in-memory data fork values that are dependent on the fork
++	 * offset in the inode. We can't assume they remain the same as attr2
++	 * has dynamic fork offsets.
++	 */
++	ifp->if_ext_max = XFS_IFORK_SIZE(ip, XFS_DATA_FORK) /
++					(uint)sizeof(xfs_bmbt_rec_t);
++	tifp->if_ext_max = XFS_IFORK_SIZE(tip, XFS_DATA_FORK) /
++					(uint)sizeof(xfs_bmbt_rec_t);
++
++	/*
+ 	 * Fix the on-disk inode values
+ 	 */
+ 	tmp = (__uint64_t)ip->i_d.di_nblocks;
+-- 
+1.6.6
+


Index: kernel.spec
===================================================================
RCS file: /cvs/pkgs/rpms/kernel/F-12/kernel.spec,v
retrieving revision 1.2007
retrieving revision 1.2008
diff -u -p -r1.2007 -r1.2008
--- kernel.spec	7 Feb 2010 20:47:07 -0000	1.2007
+++ kernel.spec	7 Feb 2010 22:05:37 -0000	1.2008
@@ -737,6 +737,9 @@ Patch3051: linux-2.6-nfs4-callback-hidde
 # btrfs
 Patch3100: linux-2.6-btrfs-fix-acl.patch
 
+# XFS
+Patch3110: xfs_swap_extents-needs-to-handle-dynamic-fork-offsets.patch
+
 # VIA Nano / VX8xx updates
 Patch11010: via-hwmon-temp-sensor.patch
 
@@ -1224,6 +1227,7 @@ ApplyPatch linux-2.6-execshield.patch
 # ext4
 
 # xfs
+ApplyPatch xfs_swap_extents-needs-to-handle-dynamic-fork-offsets.patch
 
 # btrfs
 ApplyPatch linux-2.6-btrfs-fix-acl.patch
@@ -2048,6 +2052,10 @@ fi
 # and build.
 
 %changelog
+* Sun Feb 07 2010 Kyle McMartin <kyle at redhat.com> 2.6.32.8-48.rc2
+- xfs: xfs_swap_extents needs to handle dynamic fork offsets (rhbz#510823)
+  from sandeen.
+
 * Sun Feb 07 2010 Kyle McMartin <kyle at redhat.com> 2.6.32.8-47.rc2
 - fix-ima-null-ptr-deref.patch: fix null ptr deref in IMA introduced
   in 2.6.32-rc5.