| Message ID | 164694921916.1119636.2957657161513150271.stgit@magnolia (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | xfs: make quota reservations for directory changes | expand |
On Thu, Mar 10, 2022 at 01:53:39PM -0800, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@kernel.org> > > XFS does not reserve quota for directory expansion when renaming > children into 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 rename() to exceed quota. > > Rename operations don't always expand the target directory, and we allow > a rename to proceed with no space reservation if we don't need to add a > block to the target directory to handle the addition. Moreover, the > unlink operation on the source directory generally does not expand the > directory (you'd have to free a block and then cause a btree split) and > it's probably of little consequence to leave the corner case that > renaming a file out of a directory can increase its size. > > As with link and unlink, 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. > > Because rename is its own special tricky animal, we'll patch xfs_rename > directly to reserve quota to the rename transaction. We'll leave > cleaning up the rest of xfs_rename for the metadata directory tree > patchset. > > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > --- > fs/xfs/xfs_inode.c | 33 ++++++++++++++++++++++++++++++++- > 1 file changed, 32 insertions(+), 1 deletion(-) LGTM. Reviewed-by: Dave Chinner <dchinner@redhat.com>
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c index 766a621b970d..35a2489942e5 100644 --- a/fs/xfs/xfs_inode.c +++ b/fs/xfs/xfs_inode.c @@ -3097,7 +3097,8 @@ xfs_rename( bool new_parent = (src_dp != target_dp); bool src_is_directory = S_ISDIR(VFS_I(src_ip)->i_mode); int spaceres; - int error; + bool retried = false; + int error, nospace_error = 0; trace_xfs_rename(src_dp, target_dp, src_name, target_name); @@ -3121,9 +3122,12 @@ xfs_rename( xfs_sort_for_rename(src_dp, target_dp, src_ip, target_ip, wip, inodes, &num_inodes); +retry: + nospace_error = 0; spaceres = XFS_RENAME_SPACE_RES(mp, target_name->len); error = xfs_trans_alloc(mp, &M_RES(mp)->tr_rename, spaceres, 0, 0, &tp); if (error == -ENOSPC) { + nospace_error = error; spaceres = 0; error = xfs_trans_alloc(mp, &M_RES(mp)->tr_rename, 0, 0, 0, &tp); @@ -3177,6 +3181,31 @@ xfs_rename( target_dp, target_name, target_ip, spaceres); + /* + * Try to reserve quota to handle an expansion of the target directory. + * We'll allow the rename to continue in reservationless mode if we hit + * a space usage constraint. If we trigger reservationless mode, save + * the errno if there isn't any free space in the target directory. + */ + if (spaceres != 0) { + error = xfs_trans_reserve_quota_nblks(tp, target_dp, spaceres, + 0, false); + if (error == -EDQUOT || error == -ENOSPC) { + if (!retried) { + xfs_trans_cancel(tp); + xfs_blockgc_free_quota(target_dp, 0); + retried = true; + goto retry; + } + + nospace_error = error; + spaceres = 0; + error = 0; + } + if (error) + goto out_trans_cancel; + } + /* * Check for expected errors before we dirty the transaction * so we can return an error without a transaction abort. @@ -3423,6 +3452,8 @@ xfs_rename( out_release_wip: if (wip) xfs_irele(wip); + if (error == -ENOSPC && nospace_error) + error = nospace_error; return error; }