Re: BUG: KASAN: use-after-free in free_old_xmit_skbs

From: Jason Wang <jasowang@redhat.com>
To: Cong Wang <xiyou.wangcong@gmail.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>,
	Linux Kernel Network Developers <netdev@vger.kernel.org>,
	John Fastabend <john.fastabend@gmail.com>,
	qemu-devel Developers <qemu-devel@nongnu.org>,
	virtualization@lists.linux-foundation.org,
	jean-philippe menil <jpmenil@gmail.com>
Subject: Re: BUG: KASAN: use-after-free in free_old_xmit_skbs
Date: Mon, 26 Jun 2017 10:50:53 +0800	[thread overview]
Message-ID: <9327ebf9-59db-f34e-b945-ef381ee95315@redhat.com> (raw)
In-Reply-To: <CAM_iQpXhW___-iP3kdv9MJcW=hroyp2SuMzAZdx3KCpc9-W76g@mail.gmail.com>

On 2017年06月24日 06:32, Cong Wang wrote:
> On Fri, Jun 23, 2017 at 1:43 AM, Jason Wang <jasowang@redhat.com> wrote:
>>
>> On 2017年06月23日 02:53, Michael S. Tsirkin wrote:
>>> On Thu, Jun 22, 2017 at 08:15:58AM +0200, jean-philippe menil wrote:
>>>> Hi Michael,
>>>>
>>>> from what i see, the race appear when we hit virtnet_reset in
>>>> virtnet_xdp_set.
>>>> virtnet_reset
>>>>     _remove_vq_common
>>>>       virtnet_del_vqs
>>>>         virtnet_free_queues
>>>>           kfree(vi->sq)
>>>> when the xdp program (with two instances of the program to trigger it
>>>> faster)
>>>> is added or removed.
>>>>
>>>> It's easily repeatable, with 2 cpus and 4 queues on the qemu command
>>>> line,
>>>> running the xdp_ttl tool from Jesper.
>>>>
>>>> For now, i'm able to continue my qualification, testing if xdp_qp is not
>>>> null,
>>>> but do not seem to be a sustainable trick.
>>>> if (xdp_qp && vi->xdp_queues_pairs != xdp_qp)
>>>>
>>>> Maybe it will be more clear to you with theses informations.
>>>>
>>>> Best regards.
>>>>
>>>> Jean-Philippe
>>>
>>> I'm pretty clear about the issue here, I was trying to figure out a fix.
>>> Jason, any thoughts?
>>>
>>>
>> Hi Jean:
>>
>> Does the following fix this issue? (I can't reproduce it locally through
>> xdp_ttl)
> It is tricky here.
>
>  From my understanding of the code base, the tx_lock is not sufficient
> here, because in virtnet_del_vqs() all vqs are deleted and one vp
> maps to one txq.
>
> I am afraid you have to add a spinlock somewhere to serialized
> free_old_xmit_skbs() vs. vring_del_virtqueue(). As you can see
> they are in different layers, so it is hard to figure out where to add
> it...
>
> Also, make sure we don't sleep inside the spinlock, I see a
> synchronize_net().

Looks like I miss something. I thought free_old_xmit_skbs() were 
serialized in this case since we disable all tx queues after 
netif_tx_unlock_bh()?

Jean:

I thought this could be easily reproduced by e.g produce some traffic 
and in the same time try to attach an xdp program. But looks not. How do 
you trigger this? What's your qemu command line for this?

Thanks
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization