public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Zack Weinberg" <zackw@stanford.edu>
To: linux-kernel@vger.kernel.org
Subject: 2.2 PATCH: check return from copy_*_user in fs/pipe.c
Date: Tue, 19 Jun 2001 19:08:35 -0700	[thread overview]
Message-ID: <20010619184802.D5679@stanford.edu> (raw)

The anonymous pipe code in 2.2 does not check the return value of
copy_*_user.  This can lead to silent loss of data.

The appended patch fixes the bug.  It has been in continuous use on my
machine since May 13 (2.2.19) with no problems.  It will apply to any
2.2 kernel from at least 2.2.18, there have been no changes to pipe.c
since then.

2.4 pipe.c has been completely rewritten and does not have the bug.

-- 
zw         > Having been converted to bagels in the US, ...
           Good Lord! You must have -really- offended a Jewish wizard.
           	-- Niall McAuley and Kip Williams in rec.arts.sf.fandom

--- pipe.c.zw	Tue Jun 19 18:37:13 2001
+++ pipe.c	Tue Jun 19 18:37:16 2001
@@ -31,10 +31,9 @@
 			 size_t count, loff_t *ppos)
 {
 	struct inode * inode = filp->f_dentry->d_inode;
-	ssize_t chars = 0, size = 0, read = 0;
+	ssize_t chars = 0, size = 0, read = 0, nleft = 0;
         char *pipebuf;
 
-
 	if (ppos != &filp->f_pos)
 		return -ESPIPE;
 
@@ -59,19 +58,22 @@
 		interruptible_sleep_on(&PIPE_WAIT(*inode));
 	}
 	PIPE_LOCK(*inode)++;
-	while (count>0 && (size = PIPE_SIZE(*inode))) {
+	while (count>0 && (size = PIPE_SIZE(*inode)) && nleft == 0) {
 		chars = PIPE_MAX_RCHUNK(*inode);
 		if (chars > count)
 			chars = count;
 		if (chars > size)
 			chars = size;
-		read += chars;
                 pipebuf = PIPE_BASE(*inode)+PIPE_START(*inode);
+
+		nleft = copy_to_user(buf, pipebuf, chars);
+		chars -= nleft;
+
+		read += chars;
 		PIPE_START(*inode) += chars;
 		PIPE_START(*inode) &= (PIPE_BUF-1);
 		PIPE_LEN(*inode) -= chars;
 		count -= chars;
-		copy_to_user(buf, pipebuf, chars );
 		buf += chars;
 	}
 	PIPE_LOCK(*inode)--;
@@ -80,6 +82,8 @@
 		UPDATE_ATIME(inode);
 		return read;
 	}
+	if (nleft)
+		return -EFAULT;
 	if (PIPE_WRITERS(*inode))
 		return -EAGAIN;
 	return 0;
@@ -89,7 +93,7 @@
 			  size_t count, loff_t *ppos)
 {
 	struct inode * inode = filp->f_dentry->d_inode;
-	ssize_t chars = 0, free = 0, written = 0, err=0;
+	ssize_t chars = 0, free = 0, written = 0, err = 0, nleft = 0;
 	char *pipebuf;
 
 	if (ppos != &filp->f_pos)
@@ -127,29 +131,38 @@
 			interruptible_sleep_on(&PIPE_WAIT(*inode));
 		}
 		PIPE_LOCK(*inode)++;
-		while (count>0 && (free = PIPE_FREE(*inode))) {
+		while (count>0 && (free = PIPE_FREE(*inode)) && nleft == 0) {
 			chars = PIPE_MAX_WCHUNK(*inode);
 			if (chars > count)
 				chars = count;
 			if (chars > free)
 				chars = free;
                         pipebuf = PIPE_BASE(*inode)+PIPE_END(*inode);
+
+			nleft = copy_from_user(pipebuf, buf, chars);
+			chars -= nleft;
 			written += chars;
 			PIPE_LEN(*inode) += chars;
 			count -= chars;
-			copy_from_user(pipebuf, buf, chars );
 			buf += chars;
 		}
 		PIPE_LOCK(*inode)--;
 		wake_up_interruptible(&PIPE_WAIT(*inode));
 		free = 1;
+		if (nleft) {
+			err = -EFAULT;
+			goto errout;
+		}
 	}
-	inode->i_ctime = inode->i_mtime = CURRENT_TIME;
-	mark_inode_dirty(inode);
 errout:
+	if (written) {
+		inode->i_ctime = inode->i_mtime = CURRENT_TIME;
+		mark_inode_dirty(inode);
+		err = written;
+	}
 	up(&inode->i_atomic_write);
 	down(&inode->i_sem);
-	return written ? written : err;
+	return err;
 }
 
 static long long pipe_lseek(struct file * file, long long offset, int orig)

             reply	other threads:[~2001-06-20  2:09 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2001-06-20  2:08 Zack Weinberg [this message]
2001-06-20  2:16 ` 2.2 PATCH: check return from copy_*_user in fs/pipe.c David S. Miller
2001-06-20  2:48   ` Zack Weinberg
2001-06-20  2:52     ` David S. Miller
2001-06-20  3:59       ` Zack Weinberg
2001-06-20  4:01         ` David S. Miller
2001-06-20  5:14         ` Linus Torvalds
2001-06-20  4:33   ` Andrew Tridgell
2001-06-20 15:52     ` Hugh Dickins
  -- strict thread matches above, loose matches on Subject: below --
2001-06-21  3:26 Zack Weinberg
2001-06-21  3:44 ` David S. Miller
2001-06-21  6:10   ` Zack Weinberg

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=20010619184802.D5679@stanford.edu \
    --to=zackw@stanford.edu \
    --cc=linux-kernel@vger.kernel.org \
    /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