Commit 76613903 authored by Brian Foster's avatar Brian Foster Committed by Darrick J. Wong
Browse files

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: default avatarBrian Foster <bfoster@redhat.com>
Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
Reviewed-by: default avatarDarrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: default avatarDarrick J. Wong <darrick.wong@oracle.com>
parent 825d75cd

fs/xfs/libxfs/xfs_attr.c

+18 −19
Original line number Diff line number Diff line
@@ -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)

fs/xfs/libxfs/xfs_attr_leaf.c

+0 −2
Original line number Diff line number Diff line
@@ -802,7 +802,6 @@ xfs_attr_shortform_to_leaf( 
	memset((char *)&nargs, 0, sizeof(nargs));

	nargs.dp = dp;

	nargs.geo = args->geo;

	nargs.firstblock = args->firstblock;

	nargs.total = args->total;

	nargs.whichfork = XFS_ATTR_FORK;

	nargs.trans = args->trans;
@@ -1005,7 +1004,6 @@ xfs_attr3_leaf_to_shortform( 
	memset((char *)&nargs, 0, sizeof(nargs));

	nargs.geo = args->geo;

	nargs.dp = dp;

	nargs.firstblock = args->firstblock;

	nargs.total = args->total;

	nargs.whichfork = XFS_ATTR_FORK;

	nargs.trans = args->trans;

fs/xfs/libxfs/xfs_attr_remote.c

+11 −7
Original line number Diff line number Diff line
@@ -480,11 +480,13 @@ xfs_attr_rmtval_set( 
		 * extent and then crash then the block may not contain the

		 * correct metadata after log recovery occurs.

		 */

		xfs_defer_init(NULL, args->trans->t_dfops, args->firstblock);

		xfs_defer_init(args->trans, args->trans->t_dfops,

			       &args->trans->t_firstblock);

		nmap = 1;

		error = xfs_bmapi_write(args->trans, dp, (xfs_fileoff_t)lblkno,

				  blkcnt, XFS_BMAPI_ATTRFORK, args->firstblock,

				  args->total, &map, &nmap);

				  blkcnt, XFS_BMAPI_ATTRFORK,

				  &args->trans->t_firstblock, args->total, &map,

				  &nmap);

		if (error)

			goto out_defer_cancel;

		xfs_defer_ijoin(args->trans->t_dfops, dp);
@@ -522,7 +524,8 @@ xfs_attr_rmtval_set( 


		ASSERT(blkcnt > 0);



		xfs_defer_init(NULL, args->trans->t_dfops, args->firstblock);

		xfs_defer_init(args->trans, args->trans->t_dfops,

			       &args->trans->t_firstblock);

		nmap = 1;

		error = xfs_bmapi_read(dp, (xfs_fileoff_t)lblkno,

				       blkcnt, &map, &nmap,
@@ -626,10 +629,11 @@ xfs_attr_rmtval_remove( 
	blkcnt = args->rmtblkcnt;

	done = 0;

	while (!done) {

		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_bunmapi(args->trans, args->dp, lblkno, blkcnt,

				    XFS_BMAPI_ATTRFORK, 1, args->firstblock,

				    &done);

				    XFS_BMAPI_ATTRFORK, 1,

				    &args->trans->t_firstblock, &done);

		if (error)

			goto out_defer_cancel;

		xfs_defer_ijoin(args->trans->t_dfops, args->dp);

fs/xfs/libxfs/xfs_bmap.c

+0 −1
Original line number Diff line number Diff line
@@ -1000,7 +1000,6 @@ xfs_bmap_add_attrfork_local( 
		memset(&dargs, 0, sizeof(dargs));

		dargs.geo = ip->i_mount->m_dir_geo;

		dargs.dp = ip;

		dargs.firstblock = &tp->t_firstblock;

		dargs.total = dargs.geo->fsbcount;

		dargs.whichfork = XFS_DATA_FORK;

		dargs.trans = tp;

fs/xfs/libxfs/xfs_da_btree.c

+3 −4
Original line number Diff line number Diff line
@@ -2059,10 +2059,9 @@ xfs_da_grow_inode_int( 
	 * Try mapping it in one filesystem block.

	 */

	nmap = 1;

	ASSERT(args->firstblock != NULL);

	error = xfs_bmapi_write(tp, dp, *bno, count,

			xfs_bmapi_aflag(w)|XFS_BMAPI_METADATA|XFS_BMAPI_CONTIG,

			args->firstblock, args->total, &map, &nmap);

			&tp->t_firstblock, args->total, &map, &nmap);

	if (error)

		return error;
@@ -2084,7 +2083,7 @@ xfs_da_grow_inode_int( 
			c = (int)(*bno + count - b);

			error = xfs_bmapi_write(tp, dp, b, c,

					xfs_bmapi_aflag(w)|XFS_BMAPI_METADATA,

					args->firstblock, args->total,

					&tp->t_firstblock, args->total,

					&mapp[mapi], &nmap);

			if (error)

				goto out_free_map;
@@ -2394,7 +2393,7 @@ xfs_da_shrink_inode( 
		 * the last block to the place we want to kill.

		 */

		error = xfs_bunmapi(tp, dp, dead_blkno, count,

				    xfs_bmapi_aflag(w), 0, args->firstblock,

				    xfs_bmapi_aflag(w), 0, &tp->t_firstblock,

				    &done);

		if (error == -ENOSPC) {

			if (w != XFS_DATA_FORK)

CRA Git | Maintained and supported by SUSTech CRA and CCSE