xfs: use ->t_firstblock in xattr ops

Similar to the dirops code, the xattr code uses an on-stack
firstblock variable for the various operations. This code rolls the
underlying transaction in various places, however, which means we
cannot simply replace the local firstblock vars with ->t_firstblock.
Doing so (without further changes) would invalidate the memory
pointed to by xfs_da_args.firstblock as soon as the first
transaction rolls.

To avoid this problem, remove xfs_da_args.firstblock and replace all
such accesses with ->t_firstblock at the same time. This ensures
that accesses to the current firstblock always occur through the
current transaction rather than a potentially invalid xfs_da_args
pointer.

Signed-off-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

1 files changed, 18 insertions, 19 deletions

diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index 8a7e2c0308c4a..153d2e29f872f 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -204,7 +204,6 @@ xfs_attr_set(
 	struct xfs_da_args	args;
 	struct xfs_defer_ops	dfops;
 	struct xfs_trans_res	tres;
-	xfs_fsblock_t		firstblock;
 	int			rsvd = (flags & ATTR_ROOT) != 0;
 	int			error, err2, local;
 
@@ -219,7 +218,6 @@ xfs_attr_set(
 
 	args.value = value;
 	args.valuelen = valuelen;
-	args.firstblock = &firstblock;
 	args.op_flags = XFS_DA_OP_ADDNAME | XFS_DA_OP_OKNOENT;
 	args.total = xfs_attr_calc_size(&args, &local);
 
@@ -253,7 +251,7 @@ xfs_attr_set(
 			rsvd ? XFS_TRANS_RESERVE : 0, &args.trans);
 	if (error)
 		return error;
-	xfs_defer_init(args.trans, &dfops, &firstblock);
+	xfs_defer_init(args.trans, &dfops, &args.trans->t_firstblock);
 
 	xfs_ilock(dp, XFS_ILOCK_EXCL);
 	error = xfs_trans_reserve_quota_nblks(args.trans, dp, args.total, 0,
@@ -392,7 +390,6 @@ xfs_attr_remove(
 	struct xfs_mount	*mp = dp->i_mount;
 	struct xfs_da_args	args;
 	struct xfs_defer_ops	dfops;
-	xfs_fsblock_t		firstblock;
 	int			error;
 
 	XFS_STATS_INC(mp, xs_attr_remove);
@@ -404,8 +401,6 @@ xfs_attr_remove(
 	if (error)
 		return error;
 
-	args.firstblock = &firstblock;
-
 	/*
 	 * we have no control over the attribute names that userspace passes us
 	 * to remove, so we have to allow the name lookup prior to attribute
@@ -427,7 +422,7 @@ xfs_attr_remove(
 			&args.trans);
 	if (error)
 		return error;
-	xfs_defer_init(args.trans, &dfops, &firstblock);
+	xfs_defer_init(args.trans, &dfops, &args.trans->t_firstblock);
 
 	xfs_ilock(dp, XFS_ILOCK_EXCL);
 	/*
@@ -598,7 +593,8 @@ xfs_attr_leaf_addname(
 		 * Commit that transaction so that the node_addname() call
 		 * can manage its own transactions.
 		 */
-		xfs_defer_init(NULL, args->trans->t_dfops, args->firstblock);
+		xfs_defer_init(args->trans, args->trans->t_dfops,
+			       &args->trans->t_firstblock);
 		error = xfs_attr3_leaf_to_node(args);
 		if (error)
 			goto out_defer_cancel;
@@ -687,8 +683,8 @@ xfs_attr_leaf_addname(
 		 * If the result is small enough, shrink it all into the inode.
 		 */
 		if ((forkoff = xfs_attr_shortform_allfit(bp, dp))) {
-			xfs_defer_init(NULL, args->trans->t_dfops,
-				       args->firstblock);
+			xfs_defer_init(args->trans, args->trans->t_dfops,
+				       &args->trans->t_firstblock);
 			error = xfs_attr3_leaf_to_shortform(bp, args, forkoff);
 			/* bp is gone due to xfs_da_shrink_inode */
 			if (error)
@@ -753,7 +749,8 @@ xfs_attr_leaf_removename(
 	 * If the result is small enough, shrink it all into the inode.
 	 */
 	if ((forkoff = xfs_attr_shortform_allfit(bp, dp))) {
-		xfs_defer_init(NULL, args->trans->t_dfops, args->firstblock);
+		xfs_defer_init(args->trans, args->trans->t_dfops,
+			       &args->trans->t_firstblock);
 		error = xfs_attr3_leaf_to_shortform(bp, args, forkoff);
 		/* bp is gone due to xfs_da_shrink_inode */
 		if (error)
@@ -882,8 +879,8 @@ restart:
 			 */
 			xfs_da_state_free(state);
 			state = NULL;
-			xfs_defer_init(NULL, args->trans->t_dfops,
-				       args->firstblock);
+			xfs_defer_init(args->trans, args->trans->t_dfops,
+				       &args->trans->t_firstblock);
 			error = xfs_attr3_leaf_to_node(args);
 			if (error)
 				goto out_defer_cancel;
@@ -910,7 +907,8 @@ restart:
 		 * in the index/blkno/rmtblkno/rmtblkcnt fields and
 		 * in the index2/blkno2/rmtblkno2/rmtblkcnt2 fields.
 		 */
-		xfs_defer_init(NULL, args->trans->t_dfops, args->firstblock);
+		xfs_defer_init(args->trans, args->trans->t_dfops,
+			       &args->trans->t_firstblock);
 		error = xfs_da3_split(state);
 		if (error)
 			goto out_defer_cancel;
@@ -1008,8 +1006,8 @@ restart:
 		 * Check to see if the tree needs to be collapsed.
 		 */
 		if (retval && (state->path.active > 1)) {
-			xfs_defer_init(NULL, args->trans->t_dfops,
-				       args->firstblock);
+			xfs_defer_init(args->trans, args->trans->t_dfops,
+				       &args->trans->t_firstblock);
 			error = xfs_da3_join(state);
 			if (error)
 				goto out_defer_cancel;
@@ -1134,7 +1132,8 @@ xfs_attr_node_removename(
 	 * Check to see if the tree needs to be collapsed.
 	 */
 	if (retval && (state->path.active > 1)) {
-		xfs_defer_init(NULL, args->trans->t_dfops, args->firstblock);
+		xfs_defer_init(args->trans, args->trans->t_dfops,
+			       &args->trans->t_firstblock);
 		error = xfs_da3_join(state);
 		if (error)
 			goto out_defer_cancel;
@@ -1166,8 +1165,8 @@ xfs_attr_node_removename(
 			goto out;
 
 		if ((forkoff = xfs_attr_shortform_allfit(bp, dp))) {
-			xfs_defer_init(NULL, args->trans->t_dfops,
-				       args->firstblock);
+			xfs_defer_init(args->trans, args->trans->t_dfops,
+				       &args->trans->t_firstblock);
 			error = xfs_attr3_leaf_to_shortform(bp, args, forkoff);
 			/* bp is gone due to xfs_da_shrink_inode */
 			if (error)