public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Eric Dumazet <dada1@cosmosbay.com>
To: Andrew Morton <akpm@osdl.org>
Cc: Nick Piggin <nickpiggin@yahoo.com.au>,
	Ingo Molnar <mingo@elte.hu>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Jens Axboe <jens.axboe@oracle.com>,
	tytso@mit.edu
Subject: [PATCH] splice : Must fully check for fifos
Date: Thu, 2 Nov 2006 18:05:07 +0100	[thread overview]
Message-ID: <200611021805.07962.dada1@cosmosbay.com> (raw)
In-Reply-To: <200611021802.28519.dada1@cosmosbay.com>

[-- Attachment #1: Type: text/plain, Size: 754 bytes --]

With the patch this time :( Sorry guys

Hi Andrew

I think this patch is necessary. It's quite easy to crash a 2.6.19-rc4 box :(

AFAIK the problem come from inode-diet (by Theodore Ts'o, (2006/Sep/27))

Thank you

[PATCH] splice : Must fully check for FIFO

It appears that i_pipe, i_cdev and i_bdev share the same memory location 
(anonymous union in struct inode) since commits 
577c4eb09d1034d0739e3135fd2cff50588024be
eaf796e7ef6014f208c409b2b14fddcfaafe7e3a

Because of that, testing i_pipe being NULL is not anymore sufficient to tell 
if an inode is a FIFO or not.

Therefore, we must use the S_ISFIFO(inode->i_mode) test before assuming i_pipe 
pointer is pointing to a struct pipe_inode_info.

Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>

[-- Attachment #2: splice_fix.patch --]
[-- Type: text/plain, Size: 2262 bytes --]

--- linux-2.6.19-rc4/fs/splice.c	2006-11-02 17:14:55.000000000 +0100
+++ linux-2.6.19-rc4-ed/fs/splice.c	2006-11-02 17:38:29.000000000 +0100
@@ -1115,12 +1115,14 @@
 		      struct file *out, loff_t __user *off_out,
 		      size_t len, unsigned int flags)
 {
+	struct inode *inode;
 	struct pipe_inode_info *pipe;
 	loff_t offset, *off;
 	long ret;
 
-	pipe = in->f_dentry->d_inode->i_pipe;
-	if (pipe) {
+	inode = in->f_dentry->d_inode;
+	pipe = inode->i_pipe;
+	if (pipe && S_ISFIFO(inode->i_mode)) {
 		if (off_in)
 			return -ESPIPE;
 		if (off_out) {
@@ -1140,8 +1142,9 @@
 		return ret;
 	}
 
-	pipe = out->f_dentry->d_inode->i_pipe;
-	if (pipe) {
+	inode = out->f_dentry->d_inode;
+	pipe = inode->i_pipe;
+	if (pipe && S_ISFIFO(inode->i_mode)) {
 		if (off_out)
 			return -ESPIPE;
 		if (off_in) {
@@ -1298,7 +1301,8 @@
 static long do_vmsplice(struct file *file, const struct iovec __user *iov,
 			unsigned long nr_segs, unsigned int flags)
 {
-	struct pipe_inode_info *pipe = file->f_dentry->d_inode->i_pipe;
+	struct inode *inode = file->f_dentry->d_inode;
+	struct pipe_inode_info *pipe = inode->i_pipe;
 	struct page *pages[PIPE_BUFFERS];
 	struct partial_page partial[PIPE_BUFFERS];
 	struct splice_pipe_desc spd = {
@@ -1308,7 +1312,7 @@
 		.ops = &user_page_pipe_buf_ops,
 	};
 
-	if (unlikely(!pipe))
+	if (unlikely(!pipe || !S_ISFIFO(inode->i_mode)))
 		return -EBADF;
 	if (unlikely(nr_segs > UIO_MAXIOV))
 		return -EINVAL;
@@ -1535,11 +1539,21 @@
 static long do_tee(struct file *in, struct file *out, size_t len,
 		   unsigned int flags)
 {
-	struct pipe_inode_info *ipipe = in->f_dentry->d_inode->i_pipe;
-	struct pipe_inode_info *opipe = out->f_dentry->d_inode->i_pipe;
+	struct inode *in_inode = in->f_dentry->d_inode;
+	struct inode *out_inode = out->f_dentry->d_inode;
+	struct pipe_inode_info *ipipe;
+	struct pipe_inode_info *opipe;
 	int ret = -EINVAL;
 
 	/*
+	 * CAUTION : As i_pipe/i_bdev/i_cdev share the same location,
+	 * we must check we deal with fifos/pipes, not cdev or bdev.
+	 */
+	if (!S_ISFIFO(in_inode->i_mode) || !S_ISFIFO(out_inode->i_mode))
+		return ret;
+	ipipe = in_inode->i_pipe;
+	opipe = out_inode->i_pipe;
+	/*
 	 * Duplicate the contents of ipipe to opipe without actually
 	 * copying the data.
 	 */

  reply	other threads:[~2006-11-02 17:05 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-10-30  9:03 [PATCH 1/2] lockdep: spin_lock_irqsave_nested() Peter Zijlstra
2006-10-30  9:06 ` [PATCH 2/2] lockdep: annotate bcsp driver Peter Zijlstra
2006-10-30  9:06   ` Ingo Molnar
2006-10-30  9:30   ` Marcel Holtmann
2006-10-30  9:31   ` [PATCH 2/2] lockdep: annotate bcsp driver - v2 Peter Zijlstra
2006-10-30  9:07 ` [PATCH 1/2] lockdep: spin_lock_irqsave_nested() Ingo Molnar
2006-10-30 13:12 ` Jarek Poplawski
2006-10-30 13:27   ` Jarek Poplawski
2006-10-30 13:40   ` [PATCH 1/2] lockdep: spin_lock_irqsave_nested() -v2 Peter Zijlstra
2006-10-30 14:12     ` Jarek Poplawski
2006-10-31  6:48 ` [PATCH 1/2] lockdep: spin_lock_irqsave_nested() Andrew Morton
2006-10-31  7:25   ` [PATCH] splice : two smp_mb() can be omitted Eric Dumazet
2006-10-31  7:32     ` Jens Axboe
2006-10-31  7:41       ` Eric Dumazet
2006-10-31  7:46         ` Jens Axboe
2006-10-31  9:40     ` Nick Piggin
2006-10-31  9:49       ` Jens Axboe
2006-10-31 10:51       ` Eric Dumazet
2006-10-31 22:16         ` Nick Piggin
2006-10-31 23:08           ` Eric Dumazet
2006-10-31 23:45             ` Nick Piggin
2006-11-02 17:02         ` [PATCH] splice : Must fully check for fifos Eric Dumazet
2006-11-02 17:05           ` Eric Dumazet [this message]
2006-11-02 19:07             ` Jens Axboe
2006-11-03  8:50               ` Jens Axboe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200611021805.07962.dada1@cosmosbay.com \
    --to=dada1@cosmosbay.com \
    --cc=akpm@osdl.org \
    --cc=jens.axboe@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=nickpiggin@yahoo.com.au \
    --cc=tytso@mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox