2.2 PATCH: check return from copy_*_user in fs/pipe.c

Zack Weinberg (zackw@stanford.edu)
Tue, 19 Jun 2001 19:08:35 -0700


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) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/