[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.

2 files changed, 42 insertions, 15 deletions

diff --git a/include/linux/uio.h b/include/linux/uio.h
index ec098c8e6793..85b2f0ec9d3f 100644
--- a/include/linux/uio.h
+++ b/include/linux/uio.h
@@ -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)
 {
diff --git a/mm/filemap.c b/mm/filemap.c
index ea37b24135aa..4cfb5939082e 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -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))