[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20101101082749.GA25860@redhat.com>
Date: Mon, 1 Nov 2010 10:27:49 +0200
From: "Michael S. Tsirkin" <mst@...hat.com>
To: unlisted-recipients:; (no To-header on input)
Cc: "David S. Miller" <davem@...emloft.net>,
"Michael S. Tsirkin" <mst@...hat.com>,
Herbert Xu <herbert@...dor.hengli.com.au>,
Eric Dumazet <eric.dumazet@...il.com>,
Joe Perches <joe@...ches.com>, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: [PATCH RFC] tun: remove of user-controlled memory allocation
Untested, this is just an RFC.
tun does a kmalloc where userspace controls the length. This will
produce warnings in kernel log when the length is too large, or might
block for a long while. A simple fix is to avoid the allocatiuon
altogether, and copy from user in a loop.
However, with this patch an illegal address passed to the ioctl might
leave the filter disabled. Is this something we care about? If
yes we could recover by creating a copy of the filter. Thoughts?
Signed-off-by: Michael S. Tsirkin <mst@...hat.com>
---
drivers/net/tun.c | 30 ++++++++++++++----------------
1 files changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 55f3a3e..ea36888 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -220,28 +220,23 @@ static unsigned int addr_hash_test(const u32 *mask, const u8 *addr)
static int update_filter(struct tap_filter *filter, void __user *arg)
{
- struct { u8 u[ETH_ALEN]; } *addr;
+ struct { u8 u[ETH_ALEN]; } __user *addr;
struct tun_filter uf;
- int err, alen, n, nexact;
+ int err = -EFAULT, n, nexact;
if (copy_from_user(&uf, arg, sizeof(uf)))
- return -EFAULT;
+ goto done;
if (!uf.count) {
/* Disabled */
filter->count = 0;
- return 0;
+ err = 0;
+ goto done;
}
- alen = ETH_ALEN * uf.count;
- addr = kmalloc(alen, GFP_KERNEL);
- if (!addr)
- return -ENOMEM;
-
- if (copy_from_user(addr, arg + sizeof(uf), alen)) {
- err = -EFAULT;
+ addr = arg + sizeof(uf);
+ if (!access_ok(VERIFY_READ, addr, ETH_ALEN * uf.count))
goto done;
- }
/* The filter is updated without holding any locks. Which is
* perfectly safe. We disable it first and in the worst
@@ -251,7 +246,8 @@ static int update_filter(struct tap_filter *filter, void __user *arg)
/* Use first set of addresses as an exact filter */
for (n = 0; n < uf.count && n < FLT_EXACT_COUNT; n++)
- memcpy(filter->addr[n], addr[n].u, ETH_ALEN);
+ if (__copy_from_user(filter->addr[n], addr[n].u, ETH_ALEN))
+ goto done;
nexact = n;
@@ -259,11 +255,14 @@ static int update_filter(struct tap_filter *filter, void __user *arg)
* unicast will leave the filter disabled. */
memset(filter->mask, 0, sizeof(filter->mask));
for (; n < uf.count; n++) {
- if (!is_multicast_ether_addr(addr[n].u)) {
+ u8 u[ETH_ALEN];
+ if (__copy_from_user(u, addr[n].u, ETH_ALEN))
+ goto done;
+ if (!is_multicast_ether_addr(u)) {
err = 0; /* no filter */
goto done;
}
- addr_hash_set(filter->mask, addr[n].u);
+ addr_hash_set(filter->mask, u);
}
/* For ALLMULTI just set the mask to all ones.
@@ -279,7 +278,6 @@ static int update_filter(struct tap_filter *filter, void __user *arg)
err = nexact;
done:
- kfree(addr);
return err;
}
--
1.7.3.2.91.g446ac
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists