From: Brian Foster Date: Fri, 5 Oct 2018 02:36:08 +0000 (-0500) Subject: xfs: use ->t_firstblock in xattr ops X-Git-Tag: v4.19.0-rc0~66 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=d2391b24ebcaada6d798996104a1abf4abcaf6e5;p=thirdparty%2Fxfsprogs-dev.git xfs: use ->t_firstblock in xattr ops Source kernel commit: 766139032f95bb41031f6de9c2ee0538bd035229 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 Reviewed-by: Christoph Hellwig Reviewed-by: Darrick J. Wong Signed-off-by: Darrick J. Wong Signed-off-by: Eric Sandeen --- diff --git a/libxfs/xfs_attr.c b/libxfs/xfs_attr.c index 92e445dfe..2b6e0cf33 100644 --- a/libxfs/xfs_attr.c +++ b/libxfs/xfs_attr.c @@ -199,7 +199,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; @@ -214,7 +213,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); @@ -248,7 +246,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, @@ -387,7 +385,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); @@ -399,8 +396,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 @@ -422,7 +417,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); /* @@ -593,7 +588,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; @@ -682,8 +678,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) @@ -748,7 +744,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) @@ -877,8 +874,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; @@ -905,7 +902,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; @@ -1003,8 +1001,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; @@ -1129,7 +1127,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; @@ -1161,8 +1160,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) diff --git a/libxfs/xfs_attr_leaf.c b/libxfs/xfs_attr_leaf.c index a2e2fb1a7..abb158b26 100644 --- a/libxfs/xfs_attr_leaf.c +++ b/libxfs/xfs_attr_leaf.c @@ -797,7 +797,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; @@ -1000,7 +999,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; diff --git a/libxfs/xfs_attr_remote.c b/libxfs/xfs_attr_remote.c index bc5fc3c8c..1d39ace1d 100644 --- a/libxfs/xfs_attr_remote.c +++ b/libxfs/xfs_attr_remote.c @@ -475,11 +475,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); @@ -517,7 +519,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, @@ -621,10 +624,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); diff --git a/libxfs/xfs_bmap.c b/libxfs/xfs_bmap.c index f541f5696..5964501c0 100644 --- a/libxfs/xfs_bmap.c +++ b/libxfs/xfs_bmap.c @@ -991,7 +991,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; diff --git a/libxfs/xfs_da_btree.c b/libxfs/xfs_da_btree.c index cd66272df..d736edaa4 100644 --- a/libxfs/xfs_da_btree.c +++ b/libxfs/xfs_da_btree.c @@ -2054,10 +2054,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; @@ -2079,7 +2078,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; @@ -2389,7 +2388,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) diff --git a/libxfs/xfs_da_btree.h b/libxfs/xfs_da_btree.h index 6b8a04f3f..59e290ef3 100644 --- a/libxfs/xfs_da_btree.h +++ b/libxfs/xfs_da_btree.h @@ -57,7 +57,6 @@ typedef struct xfs_da_args { xfs_dahash_t hashval; /* hash value of name */ xfs_ino_t inumber; /* input/output inode number */ struct xfs_inode *dp; /* directory inode to manipulate */ - xfs_fsblock_t *firstblock; /* ptr to firstblock for bmap calls */ struct xfs_trans *trans; /* current trans (changes over time) */ xfs_extlen_t total; /* total blocks needed, for 1st bmap */ int whichfork; /* data or attribute fork */ diff --git a/libxfs/xfs_dir2.c b/libxfs/xfs_dir2.c index aff28ed44..0e2f59fd2 100644 --- a/libxfs/xfs_dir2.c +++ b/libxfs/xfs_dir2.c @@ -270,7 +270,6 @@ xfs_dir_createname( args->total = total; args->whichfork = XFS_DATA_FORK; args->trans = tp; - args->firstblock = &tp->t_firstblock; args->op_flags = XFS_DA_OP_ADDNAME | XFS_DA_OP_OKNOENT; if (!inum) args->op_flags |= XFS_DA_OP_JUSTCHECK; @@ -437,7 +436,6 @@ xfs_dir_removename( args->hashval = dp->i_mount->m_dirnameops->hashname(name); args->inumber = ino; args->dp = dp; - args->firstblock = &tp->t_firstblock; args->total = total; args->whichfork = XFS_DATA_FORK; args->trans = tp; @@ -500,7 +498,6 @@ xfs_dir_replace( args->hashval = dp->i_mount->m_dirnameops->hashname(name); args->inumber = inum; args->dp = dp; - args->firstblock = &tp->t_firstblock; args->total = total; args->whichfork = XFS_DATA_FORK; args->trans = tp; @@ -658,7 +655,7 @@ xfs_dir2_shrink_inode( /* Unmap the fsblock(s). */ error = xfs_bunmapi(tp, dp, da, args->geo->fsbcount, 0, 0, - args->firstblock, &done); + &tp->t_firstblock, &done); if (error) { /* * ENOSPC actually can happen if we're in a removename with no diff --git a/repair/phase6.c b/repair/phase6.c index f44fe6e43..7ec206b56 100644 --- a/repair/phase6.c +++ b/repair/phase6.c @@ -1489,7 +1489,6 @@ dir2_kill_block( libxfs_defer_init(tp, &dfops, &firstblock); args.dp = ip; args.trans = tp; - args.firstblock = &firstblock; args.whichfork = XFS_DATA_FORK; args.geo = mp->m_dir_geo; if (da_bno >= mp->m_dir_geo->leafblk && da_bno < mp->m_dir_geo->freeblk)