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.
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);
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.h...
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
On 13/02/2023 20:37, Jeff Layton wrote:
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.htmlThat's arguably a bug in the spec. It really does need to do those things.
Yeah.
Also the kernel fuse code and libfuse also need to be improved to make ceph-fuse work.
Thanks Jeff.
- Xiubo
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
On Mon, 2023-02-13 at 20:59 +0800, Xiubo Li wrote:
On 13/02/2023 20:37, Jeff Layton wrote:
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.htmlThat's arguably a bug in the spec. It really does need to do those things.
Yeah.
Actually, posix_fallocate doesn't do hole punching. It just ensures that blocks are reserved to back future writes. Given that that's not something "observable" and won't change the contents of the file, then there really is no need to change the times and clear set{u,g}id bits there.
Linux' fallocate is different. It's a GNU API not covered by POSIX, and can result in an observable change to the contents of the file. There, we _must_ clear the setuid/setgid bits and update timestamps, at least in the cases where the content can change.
Also the kernel fuse code and libfuse also need to be improved to make ceph-fuse work.
Thanks Jeff.
- Xiubo
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
On 13/02/2023 21:09, Jeff Layton wrote:
On Mon, 2023-02-13 at 20:59 +0800, Xiubo Li wrote:
On 13/02/2023 20:37, Jeff Layton wrote:
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.htmlThat's arguably a bug in the spec. It really does need to do those things.
Yeah.
Actually, posix_fallocate doesn't do hole punching. It just ensures that blocks are reserved to back future writes. Given that that's not something "observable" and won't change the contents of the file, then there really is no need to change the times and clear set{u,g}id bits there.
Linux' fallocate is different. It's a GNU API not covered by POSIX, and can result in an observable change to the contents of the file. There, we _must_ clear the setuid/setgid bits and update timestamps, at least in the cases where the content can change.
Okay, get it.
Thank you Jeff for detailed explaining about this.
- Xiubo
Also the kernel fuse code and libfuse also need to be improved to make ceph-fuse work.
Thanks Jeff.
- Xiubo
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
linux-stable-mirror@lists.linaro.org
-
Jeff Layton -
xiubli＠redhat.com -
Xiubo Li