Skip to content

Commit 871b931

Browse files
author
Darrick J. Wong
committed
xfs: reserve quota for dir expansion when linking/unlinking files
XFS does not reserve quota for directory expansion when linking or unlinking children from a directory. This means that we don't reject the expansion with EDQUOT when we're at or near a hard limit, which means that unprivileged userspace can use link()/unlink() to exceed quota. The fix for this is nuanced -- link operations don't always expand the directory, and we allow a link to proceed with no space reservation if we don't need to add a block to the directory to handle the addition. Unlink operations generally do not expand the directory (you'd have to free a block and then cause a btree split) and we can defer the directory block freeing if there is no space reservation. Moreover, there is a further bug in that we do not trigger the blockgc workers to try to clear space when we're out of quota. To fix both cases, create a new xfs_trans_alloc_dir function that allocates the transaction, locks and joins the inodes, and reserves quota for the directory. If there isn't sufficient space or quota, we'll switch the caller to reservationless mode. This should prevent quota usage overruns with the least restriction in functionality. Signed-off-by: Darrick J. Wong <djwong@kernel.org> Reviewed-by: Dave Chinner <dchinner@redhat.com>
1 parent dd3b015 commit 871b931

3 files changed

Lines changed: 106 additions & 29 deletions

File tree

fs/xfs/xfs_inode.c

Lines changed: 17 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1217,7 +1217,7 @@ xfs_link(
12171217
{
12181218
xfs_mount_t *mp = tdp->i_mount;
12191219
xfs_trans_t *tp;
1220-
int error;
1220+
int error, nospace_error = 0;
12211221
int resblks;
12221222

12231223
trace_xfs_link(tdp, target_name);
@@ -1236,19 +1236,11 @@ xfs_link(
12361236
goto std_return;
12371237

12381238
resblks = XFS_LINK_SPACE_RES(mp, target_name->len);
1239-
error = xfs_trans_alloc(mp, &M_RES(mp)->tr_link, resblks, 0, 0, &tp);
1240-
if (error == -ENOSPC) {
1241-
resblks = 0;
1242-
error = xfs_trans_alloc(mp, &M_RES(mp)->tr_link, 0, 0, 0, &tp);
1243-
}
1239+
error = xfs_trans_alloc_dir(tdp, &M_RES(mp)->tr_link, sip, &resblks,
1240+
&tp, &nospace_error);
12441241
if (error)
12451242
goto std_return;
12461243

1247-
xfs_lock_two_inodes(sip, XFS_ILOCK_EXCL, tdp, XFS_ILOCK_EXCL);
1248-
1249-
xfs_trans_ijoin(tp, sip, XFS_ILOCK_EXCL);
1250-
xfs_trans_ijoin(tp, tdp, XFS_ILOCK_EXCL);
1251-
12521244
error = xfs_iext_count_may_overflow(tdp, XFS_DATA_FORK,
12531245
XFS_IEXT_DIR_MANIP_CNT(mp));
12541246
if (error)
@@ -1306,6 +1298,8 @@ xfs_link(
13061298
error_return:
13071299
xfs_trans_cancel(tp);
13081300
std_return:
1301+
if (error == -ENOSPC && nospace_error)
1302+
error = nospace_error;
13091303
return error;
13101304
}
13111305

@@ -2755,6 +2749,7 @@ xfs_remove(
27552749
xfs_mount_t *mp = dp->i_mount;
27562750
xfs_trans_t *tp = NULL;
27572751
int is_dir = S_ISDIR(VFS_I(ip)->i_mode);
2752+
int dontcare;
27582753
int error = 0;
27592754
uint resblks;
27602755

@@ -2772,31 +2767,24 @@ xfs_remove(
27722767
goto std_return;
27732768

27742769
/*
2775-
* We try to get the real space reservation first,
2776-
* allowing for directory btree deletion(s) implying
2777-
* possible bmap insert(s). If we can't get the space
2778-
* reservation then we use 0 instead, and avoid the bmap
2779-
* btree insert(s) in the directory code by, if the bmap
2780-
* insert tries to happen, instead trimming the LAST
2781-
* block from the directory.
2770+
* We try to get the real space reservation first, allowing for
2771+
* directory btree deletion(s) implying possible bmap insert(s). If we
2772+
* can't get the space reservation then we use 0 instead, and avoid the
2773+
* bmap btree insert(s) in the directory code by, if the bmap insert
2774+
* tries to happen, instead trimming the LAST block from the directory.
2775+
*
2776+
* Ignore EDQUOT and ENOSPC being returned via nospace_error because
2777+
* the directory code can handle a reservationless update and we don't
2778+
* want to prevent a user from trying to free space by deleting things.
27822779
*/
27832780
resblks = XFS_REMOVE_SPACE_RES(mp);
2784-
error = xfs_trans_alloc(mp, &M_RES(mp)->tr_remove, resblks, 0, 0, &tp);
2785-
if (error == -ENOSPC) {
2786-
resblks = 0;
2787-
error = xfs_trans_alloc(mp, &M_RES(mp)->tr_remove, 0, 0, 0,
2788-
&tp);
2789-
}
2781+
error = xfs_trans_alloc_dir(dp, &M_RES(mp)->tr_remove, ip, &resblks,
2782+
&tp, &dontcare);
27902783
if (error) {
27912784
ASSERT(error != -ENOSPC);
27922785
goto std_return;
27932786
}
27942787

2795-
xfs_lock_two_inodes(dp, XFS_ILOCK_EXCL, ip, XFS_ILOCK_EXCL);
2796-
2797-
xfs_trans_ijoin(tp, dp, XFS_ILOCK_EXCL);
2798-
xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
2799-
28002788
/*
28012789
* If we're removing a directory perform some additional validation.
28022790
*/

fs/xfs/xfs_trans.c

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1210,3 +1210,89 @@ xfs_trans_alloc_ichange(
12101210
xfs_trans_cancel(tp);
12111211
return error;
12121212
}
1213+
1214+
/*
1215+
* Allocate an transaction, lock and join the directory and child inodes to it,
1216+
* and reserve quota for a directory update. If there isn't sufficient space,
1217+
* @dblocks will be set to zero for a reservationless directory update and
1218+
* @nospace_error will be set to a negative errno describing the space
1219+
* constraint we hit.
1220+
*
1221+
* The caller must ensure that the on-disk dquots attached to this inode have
1222+
* already been allocated and initialized. The ILOCKs will be dropped when the
1223+
* transaction is committed or cancelled.
1224+
*/
1225+
int
1226+
xfs_trans_alloc_dir(
1227+
struct xfs_inode *dp,
1228+
struct xfs_trans_res *resv,
1229+
struct xfs_inode *ip,
1230+
unsigned int *dblocks,
1231+
struct xfs_trans **tpp,
1232+
int *nospace_error)
1233+
{
1234+
struct xfs_trans *tp;
1235+
struct xfs_mount *mp = ip->i_mount;
1236+
unsigned int resblks;
1237+
bool retried = false;
1238+
int error;
1239+
1240+
retry:
1241+
*nospace_error = 0;
1242+
resblks = *dblocks;
1243+
error = xfs_trans_alloc(mp, resv, resblks, 0, 0, &tp);
1244+
if (error == -ENOSPC) {
1245+
*nospace_error = error;
1246+
resblks = 0;
1247+
error = xfs_trans_alloc(mp, resv, resblks, 0, 0, &tp);
1248+
}
1249+
if (error)
1250+
return error;
1251+
1252+
xfs_lock_two_inodes(dp, XFS_ILOCK_EXCL, ip, XFS_ILOCK_EXCL);
1253+
1254+
xfs_trans_ijoin(tp, dp, XFS_ILOCK_EXCL);
1255+
xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
1256+
1257+
error = xfs_qm_dqattach_locked(dp, false);
1258+
if (error) {
1259+
/* Caller should have allocated the dquots! */
1260+
ASSERT(error != -ENOENT);
1261+
goto out_cancel;
1262+
}
1263+
1264+
error = xfs_qm_dqattach_locked(ip, false);
1265+
if (error) {
1266+
/* Caller should have allocated the dquots! */
1267+
ASSERT(error != -ENOENT);
1268+
goto out_cancel;
1269+
}
1270+
1271+
if (resblks == 0)
1272+
goto done;
1273+
1274+
error = xfs_trans_reserve_quota_nblks(tp, dp, resblks, 0, false);
1275+
if (error == -EDQUOT || error == -ENOSPC) {
1276+
if (!retried) {
1277+
xfs_trans_cancel(tp);
1278+
xfs_blockgc_free_quota(dp, 0);
1279+
retried = true;
1280+
goto retry;
1281+
}
1282+
1283+
*nospace_error = error;
1284+
resblks = 0;
1285+
error = 0;
1286+
}
1287+
if (error)
1288+
goto out_cancel;
1289+
1290+
done:
1291+
*tpp = tp;
1292+
*dblocks = resblks;
1293+
return 0;
1294+
1295+
out_cancel:
1296+
xfs_trans_cancel(tp);
1297+
return error;
1298+
}

fs/xfs/xfs_trans.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,9 @@ int xfs_trans_alloc_icreate(struct xfs_mount *mp, struct xfs_trans_res *resv,
259259
int xfs_trans_alloc_ichange(struct xfs_inode *ip, struct xfs_dquot *udqp,
260260
struct xfs_dquot *gdqp, struct xfs_dquot *pdqp, bool force,
261261
struct xfs_trans **tpp);
262+
int xfs_trans_alloc_dir(struct xfs_inode *dp, struct xfs_trans_res *resv,
263+
struct xfs_inode *ip, unsigned int *dblocks,
264+
struct xfs_trans **tpp, int *nospace_error);
262265

263266
static inline void
264267
xfs_trans_set_context(

0 commit comments

Comments
 (0)