On Thu, 9 Oct 2008, Miklos Szeredi wrote:
>
> While looking at the f_pos corruption thing, I found that splice() to
> a regular file totally ignores O_APPEND. Which allows users to bypass
> the append-only restriction. Bad...
>
> The only question is how this should be solved? Should splice()
> respect O_APPEND and ignore the offset? Or should it just return
> -EINVAL?
Good catch. sendfile() has the same issue, but I don't think we ever did
sendpage() for any filesystems, so it won't ever be relevant, and this is
probably just a splice issue.
EINVAL seems the simplest thing. Should check S_IMMUTABLE too for that
matter. Possible patch appended.
I do wonder if we shouldn't just do this in rw_verify_area(). The whole
reason for that function is that we used to have all those flock checks
etc spread out all over, and some path would inevitably just miss one
check or another. It's kind of stupid to expect low-level filesystems to
do the IS_APPEND/IS_IMMUTABLE checks.
Comments?
Linus
---
fs/splice.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/fs/splice.c b/fs/splice.c
index 1bbc6f4..769b2d3 100644
--- a/fs/splice.c
+++ b/fs/splice.c
@@ -891,6 +891,7 @@ static long do_splice_from(struct pipe_inode_info *pipe, struct file *out,
loff_t *ppos, size_t len, unsigned int flags)
{
int ret;
+ struct inode *inode;
if (unlikely(!out->f_op || !out->f_op->splice_write))
return -EINVAL;
@@ -898,6 +899,12 @@ static long do_splice_from(struct pipe_inode_info *pipe, struct file *out,
if (unlikely(!(out->f_mode & FMODE_WRITE)))
return -EBADF;
+ inode = out->f_dentry->d_inode;
+ if (IS_IMMUTABLE(inode))
+ return -EPERM;
+ if (IS_APPEND(inode))
+ return -EINVAL;
+
ret = rw_verify_area(WRITE, out, ppos, len);
if (unlikely(ret < 0))
return ret;
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
