On Wed, Apr 23, 2014 at 12:06:39AM -0500, Eric Biggers wrote:
> The proposed patch doesn't work because in compat_rw_copy_check_uvector(),
> 'iov' is incremented in the loop before it is freed or returned. This
> probably should be changed to indexing with 'seg', like in the non-compat
> version...
Do'h, I am indeed blind.
Updated patch below.
Thanks,
Miklos
----
Subject: vfs: rw_copy_check_uvector() - free iov on error
From: Miklos Szeredi <mszeredi@xxxxxxx>
Some callers (aio_run_iocb, vmsplice_to_user) forget to free the iov on
error. This seems to be a recurring problem, with most callers being buggy
initially.
So instead of fixing the callers, fix the semantics: free the allocated iov
on error, so callers don't have to.
We could return either fast_pointer or NULL for the error case. I've opted
for NULL.
Found by Coverity Scan.
Signed-off-by: Miklos Szeredi <mszeredi@xxxxxxx>
---
fs/compat.c | 24 +++++++++++++++---------
fs/read_write.c | 13 ++++++++++---
2 files changed, 25 insertions(+), 12 deletions(-)
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -689,7 +689,7 @@ ssize_t rw_copy_check_uvector(int type,
}
if (copy_from_user(iov, uvector, nr_segs*sizeof(*uvector))) {
ret = -EFAULT;
- goto out;
+ goto out_free;
}
/*
@@ -710,12 +710,12 @@ ssize_t rw_copy_check_uvector(int type,
* it's about to overflow ssize_t */
if (len < 0) {
ret = -EINVAL;
- goto out;
+ goto out_free;
}
if (type >= 0
&& unlikely(!access_ok(vrfy_dir(type), buf, len))) {
ret = -EFAULT;
- goto out;
+ goto out_free;
}
if (len > MAX_RW_COUNT - ret) {
len = MAX_RW_COUNT - ret;
@@ -726,6 +726,13 @@ ssize_t rw_copy_check_uvector(int type,
out:
*ret_pointer = iov;
return ret;
+
+out_free:
+ if (iov != fast_pointer) {
+ kfree(iov);
+ iov = NULL;
+ }
+ goto out;
}
static ssize_t do_readv_writev(int type, struct file *file,
--- a/fs/compat.c
+++ b/fs/compat.c
@@ -549,7 +549,7 @@ ssize_t compat_rw_copy_check_uvector(int
struct iovec **ret_pointer)
{
compat_ssize_t tot_len;
- struct iovec *iov = *ret_pointer = fast_pointer;
+ struct iovec *iov = fast_pointer;
ssize_t ret = 0;
int seg;
@@ -570,11 +570,10 @@ ssize_t compat_rw_copy_check_uvector(int
if (iov == NULL)
goto out;
}
- *ret_pointer = iov;
ret = -EFAULT;
if (!access_ok(VERIFY_READ, uvector, nr_segs*sizeof(*uvector)))
- goto out;
+ goto out_free;
/*
* Single unix specification:
@@ -593,27 +592,34 @@ ssize_t compat_rw_copy_check_uvector(int
if (__get_user(len, &uvector->iov_len) ||
__get_user(buf, &uvector->iov_base)) {
ret = -EFAULT;
- goto out;
+ goto out_free;
}
if (len < 0) /* size_t not fitting in compat_ssize_t .. */
- goto out;
+ goto out_free;
if (type >= 0 &&
!access_ok(vrfy_dir(type), compat_ptr(buf), len)) {
ret = -EFAULT;
- goto out;
+ goto out_free;
}
if (len > MAX_RW_COUNT - tot_len)
len = MAX_RW_COUNT - tot_len;
tot_len += len;
- iov->iov_base = compat_ptr(buf);
- iov->iov_len = (compat_size_t) len;
+ iov[seg].iov_base = compat_ptr(buf);
+ iov[seg].iov_len = (compat_size_t) len;
uvector++;
- iov++;
}
ret = tot_len;
out:
+ *ret_pointer = iov;
return ret;
+
+out_free:
+ if (iov != fast_pointer) {
+ kfree(iov);
+ iov = NULL;
+ }
+ goto out;
}
static inline long
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
