Data type overflow in xfs_trans_unreserve_and_mod_sb

[Shailendra Tripathi <stripathi@agami.com>]

[-- Attachment #1: Type: text/plain, Size: 1743 bytes --]

Hi David,
           As part of fixing xfs_reserve_blocks issue, you might want to 
fix an issue in xfs_trans_unreserve_and_mod_sb as well. Since, I am on 
much older version, my patch is not applicable on newer trees. However, 
the patch is attached for your reference.

The problem is as below:

Superblock modifications required during transaction are stored in delta 
fields in transaction. These fields are applied to the superblock when 
transaction commits.

The in-core superblock changes are done in 
xfs_trans_unreserve_and_mod_sb. It calls xfs_mod_incore_sb_batch 
function to apply the changes. This function tries to apply the deltas 
and if it fails for any reason, it backs out all the changes. One 
typical modification done is like that:

         case XFS_SBS_DBLOCKS:
                 lcounter = (long long)mp->m_sb.sb_dblocks;
                 lcounter += delta;
                 if (lcounter < 0) {
                         ASSERT(0);
                         return (XFS_ERROR(EINVAL));
                 }
                 mp->m_sb.sb_dblocks = lcounter;
                 return (0);

So, when it returns EINVAL, the second part of the code backs out the 
changes made to superblock. However, the worst part is that 
xfs_trans_unreserve_and_mod_sb does not return any error value. The 
transaction appears to be committed peacefully without returning the 
error. You don't notice this unless you do I/O on the filesystem. Later, 
it hits some sort of in-memory corruption or other errors.

We hit this issue in our testing we tried to grow the filesystem from 
from 100GB to 10000GB. This is beyond the interger (31 bits) limit and, 
hence, for dblocks and fdblocks,  xfs_mod_sb struct does not pass in 
correct data.




[-- Attachment #2: xfs-sb-mod-growfs.patch --]
[-- Type: text/x-patch, Size: 3709 bytes --]

diff -urNp -X kernel-2.6.7-dontdiff kernel-2.6.7-orig/fs/xfs/xfs_mount.c kernel-2.6.7-modif/fs/xfs/xfs_mount.c
--- kernel-2.6.7-orig/fs/xfs/xfs_mount.c
+++ kernel-2.6.7-modif/fs/xfs/xfs_mount.c
@@ -1286,7 +1286,7 @@ xfs_mod_sb(xfs_trans_t *tp, __int64_t fi
  */
 STATIC int
 xfs_mod_incore_sb_unlocked(xfs_mount_t *mp, xfs_sb_field_t field,
-			int delta, int rsvd)
+			long delta, int rsvd)
 {
 	int		scounter;	/* short counter for 32 bit fields */
 	long long	lcounter;	/* long counter for 64 bit fields */
diff -urNp -X kernel-2.6.7-dontdiff kernel-2.6.7-orig/fs/xfs/xfs_mount.h kernel-2.6.7-modif/fs/xfs/xfs_mount.h
--- kernel-2.6.7-orig/fs/xfs/xfs_mount.h
+++ kernel-2.6.7-modif/fs/xfs/xfs_mount.h
@@ -551,10 +551,11 @@ static inline xfs_agblock_t XFS_DADDR_TO
 
 /*
  * This structure is for use by the xfs_mod_incore_sb_batch() routine.
+ * xfs_growfs can specify a few fields which are more than int limit
  */
 typedef struct xfs_mod_sb {
 	xfs_sb_field_t	msb_field;	/* Field to modify, see below */
-	int		msb_delta;	/* Change to make to specified field */
+	long		msb_delta;	/* Change to make to specified field */
 } xfs_mod_sb_t;
 
 #define	XFS_MOUNT_ILOCK(mp)	mutex_lock(&((mp)->m_ilock), PINOD)
diff -urNp -X kernel-2.6.7-dontdiff kernel-2.6.7-orig/fs/xfs/xfs_trans.c kernel-2.6.7-modif/fs/xfs/xfs_trans.c
--- kernel-2.6.7-orig/fs/xfs/xfs_trans.c
+++ kernel-2.6.7-modif/fs/xfs/xfs_trans.c
@@ -590,62 +590,62 @@ xfs_trans_unreserve_and_mod_sb(
 	if (tp->t_flags & XFS_TRANS_SB_DIRTY) {
 		if (tp->t_icount_delta != 0) {
 			msbp->msb_field = XFS_SBS_ICOUNT;
-			msbp->msb_delta = (int)tp->t_icount_delta;
+			msbp->msb_delta = tp->t_icount_delta;
 			msbp++;
 		}
 		if (tp->t_ifree_delta != 0) {
 			msbp->msb_field = XFS_SBS_IFREE;
-			msbp->msb_delta = (int)tp->t_ifree_delta;
+			msbp->msb_delta = tp->t_ifree_delta;
 			msbp++;
 		}
 		if (tp->t_fdblocks_delta != 0) {
 			msbp->msb_field = XFS_SBS_FDBLOCKS;
-			msbp->msb_delta = (int)tp->t_fdblocks_delta;
+			msbp->msb_delta = tp->t_fdblocks_delta;
 			msbp++;
 		}
 		if (tp->t_frextents_delta != 0) {
 			msbp->msb_field = XFS_SBS_FREXTENTS;
-			msbp->msb_delta = (int)tp->t_frextents_delta;
+			msbp->msb_delta = tp->t_frextents_delta;
 			msbp++;
 		}
 		if (tp->t_dblocks_delta != 0) {
 			msbp->msb_field = XFS_SBS_DBLOCKS;
-			msbp->msb_delta = (int)tp->t_dblocks_delta;
+			msbp->msb_delta = tp->t_dblocks_delta;
 			msbp++;
 		}
 		if (tp->t_agcount_delta != 0) {
 			msbp->msb_field = XFS_SBS_AGCOUNT;
-			msbp->msb_delta = (int)tp->t_agcount_delta;
+			msbp->msb_delta = tp->t_agcount_delta;
 			msbp++;
 		}
 		if (tp->t_imaxpct_delta != 0) {
 			msbp->msb_field = XFS_SBS_IMAX_PCT;
-			msbp->msb_delta = (int)tp->t_imaxpct_delta;
+			msbp->msb_delta = tp->t_imaxpct_delta;
 			msbp++;
 		}
 		if (tp->t_rextsize_delta != 0) {
 			msbp->msb_field = XFS_SBS_REXTSIZE;
-			msbp->msb_delta = (int)tp->t_rextsize_delta;
+			msbp->msb_delta = tp->t_rextsize_delta;
 			msbp++;
 		}
 		if (tp->t_rbmblocks_delta != 0) {
 			msbp->msb_field = XFS_SBS_RBMBLOCKS;
-			msbp->msb_delta = (int)tp->t_rbmblocks_delta;
+			msbp->msb_delta = tp->t_rbmblocks_delta;
 			msbp++;
 		}
 		if (tp->t_rblocks_delta != 0) {
 			msbp->msb_field = XFS_SBS_RBLOCKS;
-			msbp->msb_delta = (int)tp->t_rblocks_delta;
+			msbp->msb_delta = tp->t_rblocks_delta;
 			msbp++;
 		}
 		if (tp->t_rextents_delta != 0) {
 			msbp->msb_field = XFS_SBS_REXTENTS;
-			msbp->msb_delta = (int)tp->t_rextents_delta;
+			msbp->msb_delta = tp->t_rextents_delta;
 			msbp++;
 		}
 		if (tp->t_rextslog_delta != 0) {
 			msbp->msb_field = XFS_SBS_REXTSLOG;
-			msbp->msb_delta = (int)tp->t_rextslog_delta;
+			msbp->msb_delta = tp->t_rextslog_delta;
 			msbp++;
 		}
 	}