Commit d4872de3 authored by Andrew Morton's avatar Andrew Morton Committed by Linus Torvalds

[PATCH] readv/writev bounds checking fixes

- writev currently returns -EFAULT if _any_ of the segments has an
invalid address.  We should only return -EFAULT if the first segment
has a bad address.

If some of the first segments have valid addresses we need to write
them and return a partial result.

- The current code only checks if the sum-of-lengths is negative.  If
individual segments have a negative length but the result is positive
we miss that.

So rework the code to detect this, and to be immune to odd wrapping
situations.

As a bonus, we save one pass across the iovec.

- ditto for readv.

The check for "does any segment have a negative length" has already
been performed in do_readv_writev(), but it's basically free here, and
we need to do it for generic_file_read/write anyway.

This all means that the iov_length() function is unsafe because of
wrap/overflow isues.  It should only be used after the
generic_file_read/write or do_readv_writev() checking has been
performed.  Its callers have been reviewed and they are OK.

The code now passes LTP testing and has been QA'd by Janet's team.
parent bd90a275
......@@ -35,7 +35,11 @@ struct iovec
#endif
/*
* Total number of bytes covered by an iovec
* Total number of bytes covered by an iovec.
*
* NOTE that it is not safe to use this function until all the iovec's
* segment lengths have been validated. Because the individual lengths can
* overflow a size_t when added together.
*/
static inline size_t iov_length(const struct iovec *iov, unsigned long nr_segs)
{
......
......@@ -1134,10 +1134,26 @@ __generic_file_aio_read(struct kiocb *iocb, const struct iovec *iov,
struct file *filp = iocb->ki_filp;
ssize_t retval;
unsigned long seg;
size_t count = iov_length(iov, nr_segs);
size_t count;
if ((ssize_t) count < 0)
return -EINVAL;
count = 0;
for (seg = 0; seg < nr_segs; seg++) {
const struct iovec *iv = &iov[seg];
/*
* If any segment has a negative length, or the cumulative
* length ever wraps negative then return -EINVAL.
*/
count += iv->iov_len;
if (unlikely((ssize_t)(count|iv->iov_len) < 0))
return -EINVAL;
if (access_ok(VERIFY_WRITE, iv->iov_base, iv->iov_len))
continue;
if (seg == 0)
return -EFAULT;
nr_segs = seg;
break;
}
/* coalesce the iovecs and go direct-to-BIO for O_DIRECT */
if (filp->f_flags & O_DIRECT) {
......@@ -1166,11 +1182,6 @@ __generic_file_aio_read(struct kiocb *iocb, const struct iovec *iov,
goto out;
}
for (seg = 0; seg < nr_segs; seg++) {
if (!access_ok(VERIFY_WRITE,iov[seg].iov_base,iov[seg].iov_len))
return -EFAULT;
}
retval = 0;
if (count) {
for (seg = 0; seg < nr_segs; seg++) {
......@@ -2032,8 +2043,8 @@ generic_file_write_nolock(struct file *file, const struct iovec *iov,
{
struct address_space * mapping = file->f_dentry->d_inode->i_mapping;
struct address_space_operations *a_ops = mapping->a_ops;
const size_t ocount = iov_length(iov, nr_segs);
size_t count = ocount;
size_t ocount; /* original count */
size_t count; /* after file limit checks */
struct inode *inode = mapping->host;
unsigned long limit = current->rlim[RLIMIT_FSIZE].rlim_cur;
long status = 0;
......@@ -2050,13 +2061,25 @@ generic_file_write_nolock(struct file *file, const struct iovec *iov,
unsigned long seg;
char *buf;
if (unlikely((ssize_t)count < 0))
return -EINVAL;
ocount = 0;
for (seg = 0; seg < nr_segs; seg++) {
if (!access_ok(VERIFY_READ,iov[seg].iov_base,iov[seg].iov_len))
const struct iovec *iv = &iov[seg];
/*
* If any segment has a negative length, or the cumulative
* length ever wraps negative then return -EINVAL.
*/
ocount += iv->iov_len;
if (unlikely((ssize_t)(ocount|iv->iov_len) < 0))
return -EINVAL;
if (access_ok(VERIFY_READ, iv->iov_base, iv->iov_len))
continue;
if (seg == 0)
return -EFAULT;
nr_segs = seg;
break;
}
count = ocount;
pos = *ppos;
if (unlikely(pos < 0))
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment