linux-kernel - Re: splice vs O

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.LFD.2.00.0810090830170.3210@nehalem.linux-foundation.org>
Date:	Thu, 9 Oct 2008 08:37:50 -0700 (PDT)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Miklos Szeredi <miklos@...redi.hu>
cc:	jens.axboe@...cle.com, linux-kernel@...r.kernel.org,
	linux-fsdevel@...r.kernel.org
Subject: Re: splice vs O_APPEND



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-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/