Date:	Wed, 11 Sep 2013 07:43:55 +0300
From:	"Michael S. Tsirkin" <mst@...hat.com>
To:	Wannes Rombouts <wannes.rombouts@...tech.eu>
Cc:	davem@...emloft.net, jasowang@...hat.com, edumazet@...gle.com,
	nhorman@...driver.com, netdev@...r.kernel.org,
	Kevin Soules <kevin.soules@...tech.eu>
Subject: Re: Use-after-free in TUNSETIFF

On Wed, Sep 11, 2013 at 01:59:47AM +0200, Wannes Rombouts wrote:
> (I sent this email to security@...nel.org but they told me I should send
> it to you guys, so here you go.)
> 
> Hi,
> 
> I would like to report what I believe could be a potential CAP_NET_ADMIN
> to ring0 privilege escalation.
> 
> The bug is in the way tuntap interfaces are initialized, when given an
> invalid name they cause a use after free. Also software like vmware
> allows for at least a freeze or kernel panic by a simple user but might
> also allow privilege escalation.
> 
> Very simple to test, this causes a crash:
> # ip tuntap add dev %% mode tap
> If it doesn't crash immediately wait a few seconds and try again.
> 
> 
> We haven't managed to exploit the use after free yet, but we are still
> working on it. At least it crashes even with the latest kernel 3.11 and
> on different distros. (tested on Debian, Ubuntu and Arch) Looking at the
> source the bug seems quite old.
> 
> 
> Here is our analysis:
> 
> A user with CAP_NET_ADMIN calls ioctl with TUNSETIFF and an invalid name
> for example "%d%d".
> 
> tun_set_iff starts to initialize the tun_struct.
> http://lxr.free-electrons.com/source/drivers/net/tun.c#L1589
> 
> It calls tun_flow_init which starts a timer with tun_flow_cleanup as
> callback. http://lxr.free-electrons.com/source/drivers/net/tun.c#L852
> 
> After this tun_set_iff calls register_netdevice which returns an error
> because of the invalid name.
> 
> This error causes the goto err_free_dev and the call to free_netdev.
> This will free the tun_struct.
> 
> Later, once the callback gets called it uses bad memory. Sometimes it
> doesn’t get called because the timer_list has been compromised and we
> get a kernel panic at:
> http://lxr.free-electrons.com/source/kernel/timer.c?v=2.6.33#L949
> 
> But it is possible to get some memory from userland that overlaps only
> the beginning of the tun_struct without overwriting the timer_list
> because there is a big array before it. Then it might be possible to
> exploit tun_flow_cleanup when it is called, but we didn't succeed yet.
> 
> ------------------------------------------------------------------------
> 
> 
> This is the first time we try to exploit the kernel so we basically suck
> at this. I don't know if someone more skilled could do this easily or
> not, but we'll keep trying and I'll let you know if we manage it.
> 
> In the mean time please let us know what you think of this and of course
> we are very interested in the way this is patched. Please keep us in the
> loop.
> 
> Of course we will be happy to assist in any way we can, feel free to
> ask! Also we would like to know when you think it would be reasonable to
> disclose and talk about this bug.
> 
> Regards,
> 
> Wannes 'wapiflapi' Rombouts
> Kevin 'eax64' Soules
> 
> 
>

Thanks a lot for the report. So this one is easy to fix I think.
Does the below patch help?
However, looking at the error handling in that function,
it looks like it could leak resources in many other ways.
We probably need more patches on top to fix it properly.

-->

tun: cleanup flow on set_iff error path

Signed-off-by: Michael S. Tsirkin <mst@...hat.com>

---

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index b7c457a..c0470c1 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1660,11 +1660,11 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
 		INIT_LIST_HEAD(&tun->disabled);
 		err = tun_attach(tun, file);
 		if (err < 0)
-			goto err_free_dev;
+			goto err_free_flow;
 
 		err = register_netdevice(tun->dev);
 		if (err < 0)
-			goto err_free_dev;
+			goto err_free_flow;
 
 		if (device_create_file(&tun->dev->dev, &dev_attr_tun_flags) ||
 		    device_create_file(&tun->dev->dev, &dev_attr_owner) ||
@@ -1708,7 +1708,9 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
 	strcpy(ifr->ifr_name, tun->dev->name);
 	return 0;
 
- err_free_dev:
+err_free_flow:
+	tun_flow_uninit(dev);
+err_free_dev:
 	free_netdev(dev);
 	return err;
 }
 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists