On 26 Jan 2022 at 07:48, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@xxxxxxxxxx>
>
> In XFS, we always update the inode change and modification time when any
> preallocation operation succeeds. Furthermore, as various fallocate
> modes can change the file contents (extending EOF, punching holes,
> zeroing things, shifting extents), we should drop file privileges like
> suid just like we do for a regular write(). There's already a VFS
> helper that figures all this out for us, so use that.
>
> The net effect of this is that we no longer drop suid/sgid if the caller
> is root, but we also now drop file capabilities.
>
> Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
> ---
> fs/xfs/xfs_file.c | 23 +++++++++++++++++++----
> 1 file changed, 19 insertions(+), 4 deletions(-)
>
>
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index 22ad207bedf4..eee5fb20cf8d 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -1057,13 +1057,28 @@ xfs_file_fallocate(
> }
> }
>
> - if (file->f_flags & O_DSYNC)
> - flags |= XFS_PREALLOC_SYNC;
> -
Without the above change, if fallocate() is invoked with FALLOC_FL_PUNCH_HOLE,
FALLOC_FL_COLLAPSE_RANGE or FALLOC_FL_INSERT_RANGE, we used to update inode's
timestamp, remove setuid/setgid bits and then perform a synchronous
transaction commit if O_DSYNC flag is set.
However, with this patch applied, the transaction (inside
xfs_vn_update_time()) that updates file's inode contents (i.e. timestamps and
setuid/setgid bits) is not synchronous and hence the O_DSYNC flag is not
honored if the fallocate operation is one of FALLOC_FL_PUNCH_HOLE,
FALLOC_FL_COLLAPSE_RANGE or FALLOC_FL_INSERT_RANGE.
> - error = xfs_update_prealloc_flags(ip, flags);
> + /* Update [cm]time and drop file privileges like a regular write. */
> + error = file_modified(file);
> if (error)
> goto out_unlock;
>
> + /*
> + * If we need to change the PREALLOC flag, do so. We already updated
> + * the timestamps and cleared the suid flags, so we don't need to do
> + * that again. This must be committed before the size change so that
> + * we don't trim post-EOF preallocations.
> + */
> + if (flags) {
> + flags |= XFS_PREALLOC_INVISIBLE;
> +
> + if (file->f_flags & O_DSYNC)
> + flags |= XFS_PREALLOC_SYNC;
> +
> + error = xfs_update_prealloc_flags(ip, flags);
> + if (error)
> + goto out_unlock;
> + }
> +
> /* Change file size if needed */
> if (new_size) {
> struct iattr iattr;
--
chandan
