Re: [PATCH] ceph: update the time stamps and try to drop the suid/sgid

[Jeff Layton <jlayton@kernel.org>]

On Mon, 2023-02-13 at 19:10 +0800, xiubli@redhat.com wrote:
> From: Xiubo Li <xiubli@redhat.com>
> 
> The fallocate will try to clear the suid/sgid if a unprevileged user
> changed the file.
> 
> There is no Posix item requires that we should clear the suid/sgid
> in fallocate code path but this is the default behaviour for most of
> the filesystems and the VFS layer. And also the same for the write
> code path, which have already support it.
> 

Huh, you're right. It really doesn't say anything about the timestamps
or setuid bits:

    https://pubs.opengroup.org/onlinepubs/9699919799/functions/posix_fallocate.html


That's arguably a bug in the spec. It really does need to do those
things.

> And also we need to update the time stamps since the fallocate will
> change the file contents.
> 
> Cc: stable@vger.kernel.org
> URL: https://tracker.ceph.com/issues/58054
> Signed-off-by: Xiubo Li <xiubli@redhat.com>
> ---
>  fs/ceph/file.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> index 903de296f0d3..dee3b445f415 100644
> --- a/fs/ceph/file.c
> +++ b/fs/ceph/file.c
> @@ -2502,6 +2502,9 @@ static long ceph_fallocate(struct file *file, int mode,
>  	loff_t endoff = 0;
>  	loff_t size;
>  
> +	dout("%s %p %llx.%llx mode %x, offset %llu length %llu\n", __func__,
> +	     inode, ceph_vinop(inode), mode, offset, length);
> +
>  	if (mode != (FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE))
>  		return -EOPNOTSUPP;
>  
> @@ -2539,6 +2542,10 @@ static long ceph_fallocate(struct file *file, int mode,
>  	if (ret < 0)
>  		goto unlock;
>  
> +	ret = file_modified(file);
> +	if (ret)
> +		goto put_caps;
> +
>  	filemap_invalidate_lock(inode->i_mapping);
>  	ceph_fscache_invalidate(inode, false);
>  	ceph_zero_pagecache_range(inode, offset, length);
> @@ -2554,6 +2561,7 @@ static long ceph_fallocate(struct file *file, int mode,
>  	}
>  	filemap_invalidate_unlock(inode->i_mapping);
>  
> +put_caps:
>  	ceph_put_cap_refs(ci, got);
>  unlock:
>  	inode_unlock(inode);

Reviewed-by: Jeff Layton <jlayton@kernel.org>