lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Sun, 28 Apr 2019 10:49:25 -0700
From:   Cong Wang <xiyou.wangcong@...il.com>
To:     Yue Haibing <yuehaibing@...wei.com>
Cc:     David Miller <davem@...emloft.net>,
        Jason Wang <jasowang@...hat.com>,
        Eric Dumazet <edumazet@...gle.com>,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        "Michael S. Tsirkin" <mst@...hat.com>,
        "Li,Rongqing" <lirongqing@...du.com>,
        Nicolas Dichtel <nicolas.dichtel@...nd.com>,
        Chas Williams <3chas3@...il.com>, wangli39@...du.com,
        LKML <linux-kernel@...r.kernel.org>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>
Subject: Re: [PATCH] tun: Fix use-after-free in tun_net_xmit

On Sat, Apr 27, 2019 at 8:06 PM Yue Haibing <yuehaibing@...wei.com> wrote:
>
> If tun driver have multiqueues, user close the last queue by
> tun_detach, then tun->tfiles[index] is not cleared. Then a new
> queue may add to the tun, which using rcu_assign_pointer
> tun->tfiles[index] to the new tfile and increase the numqueues.
> However if there send a packet during this time, which picking the last
> queue, it may uses the old tun->tfiles[index], beacause there no
> RCU grace period.

This analysis makes sense. It is a normal scenario for RCU,
where readers could still read even after we unpublish the RCU
protected structure, we only need to worry about when we free it.


> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index e9ca1c0..3770aba 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -876,6 +876,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file,
>          */
>         rcu_assign_pointer(tfile->tun, tun);
>         rcu_assign_pointer(tun->tfiles[tun->numqueues], tfile);
> +       synchronize_net();
>         tun->numqueues++;
>         tun_set_real_num_queues(tun);

But this fix doesn't make any sense, we only wait for RCU
grace period when freeing old ones, not for new ones. RCU
grace period is all about readers against free.

This is why I came up with the SOCK_RCU_FREE patch, which
is also blocking-free.


Thanks.

Powered by blists - more mailing lists