On 8/15/19 10:18 AM, Yang Yingliang wrote:
> I got a UAF repport in tun driver when doing fuzzy test:
>
>
> [ 466.368604] page:ffffea000dc84e00 refcount:1 mapcount:0
> mapping:ffff8883df1b4f00 index:0x0 compound_mapcount: 0
> [ 466.371582] flags: 0x2fffff80010200(slab|head)
> [ 466.372910] raw: 002fffff80010200 dead000000000100 dead000000000122
> ffff8883df1b4f00
> [ 466.375209] raw: 0000000000000000 0000000000070007 00000001ffffffff
> 0000000000000000
> [ 466.377778] page dumped because: kasan: bad access detected
> [ 466.379730]
> [ 466.380288] Memory state around the buggy address:
> [ 466.381844] ffff888372139100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> fb fb
> [ 466.384009] ffff888372139180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> fb fb
> [ 466.386131] >ffff888372139200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> fb fb
> [ 466.388257] ^
> [ 466.390234] ffff888372139280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> fb fb
> [ 466.392512] ffff888372139300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> fb fb
> [ 466.394667]
> ==================================================================
>
> tun_chr_read_iter() accessed the memory which freed by free_netdev()
> called by tun_set_iff():
>
> CPUA CPUB
> tun_set_iff()
> alloc_netdev_mqs()
> tun_attach()
> tun_chr_read_iter()
> tun_get()
> register_netdevice()
> tun_detach_all()
> synchronize_net()
> tun_do_read()
> tun_ring_recv()
> schedule()
> free_netdev()
> tun_put() <-- UAF
UAF on what exactly ? The dev_hold() should prevent the free_netdev().
>
> Set a new bit in tun->flag if register_netdevice() successed,
> without this bit, tun_get() returns NULL to avoid using a
> freed tun pointer.
>
> Fixes: eb0fb363f920 ("tuntap: attach queue 0 before registering netdevice")
> Reported-by: Hulk Robot <hul...@huawei.com>
> Signed-off-by: Yang Yingliang <yangyingli...@huawei.com>
> ---
> drivers/net/tun.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index db16d7a13e00..cbd60c276c40 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -115,6 +115,7 @@ do {
> \
> /* High bits in flags field are unused. */
> #define TUN_VNET_LE 0x80000000
> #define TUN_VNET_BE 0x40000000
> +#define TUN_DEV_REGISTERED 0x20000000
>
> #define TUN_FEATURES (IFF_NO_PI | IFF_ONE_QUEUE | IFF_VNET_HDR | \
> IFF_MULTI_QUEUE | IFF_NAPI | IFF_NAPI_FRAGS)
> @@ -719,8 +720,10 @@ static void __tun_detach(struct tun_file *tfile, bool
> clean)
> netif_carrier_off(tun->dev);
>
> if (!(tun->flags & IFF_PERSIST) &&
> - tun->dev->reg_state == NETREG_REGISTERED)
> + tun->dev->reg_state == NETREG_REGISTERED) {
> unregister_netdevice(tun->dev);
> + tun->flags &= ~TUN_DEV_REGISTERED;
Isn't this done too late ?
> + }
> }
> if (tun)
> xdp_rxq_info_unreg(&tfile->xdp_rxq);
> @@ -884,8 +887,10 @@ static struct tun_struct *tun_get(struct tun_file *tfile)
>
> rcu_read_lock();
> tun = rcu_dereference(tfile->tun);
> - if (tun)
> + if (tun && (tun->flags & TUN_DEV_REGISTERED))
> dev_hold(tun->dev);
> + else
> + tun = NULL;
> rcu_read_unlock();
>
> return tun;
> @@ -2836,6 +2841,7 @@ static int tun_set_iff(struct net *net, struct file
> *file, struct ifreq *ifr)
> err = register_netdevice(tun->dev);
> if (err < 0)
> goto err_detach;
> + tun->flags |= TUN_DEV_REGISTERED;
> }
>
> netif_carrier_on(tun->dev);
>
So tun_get() will return NULL as long as tun_set_iff() (TUNSETIFF ioctl()) has
not yet been called ?
This could break some applications, since tun_get() is used from poll() and
other syscalls.
