Date: Sun, 08 Jul 2012 23:40:30 +0300 From: Mikolaj Golub <trociny@freebsd.org> To: "Bjoern A. Zeeb" <bzeeb-lists@lists.zabbadoz.net> Cc: d@delphij.net, FreeBSD virtualization mailing list <freebsd-virtualization@FreeBSD.org> Subject: Re: GPF when doing jail -r, possibly an use-after-free Message-ID: <86obnqq94x.fsf@kopusha.home.net> In-Reply-To: <E909B0C0-F4DE-4110-B151-98FAC9330B82@lists.zabbadoz.net> (Bjoern A. Zeeb's message of "Sat, 7 Jul 2012 20:38:23 %2B0000") References: <4FF32FC4.6020701@delphij.net> <86wr2kau38.fsf@in138.ua3> <4FF5E87C.2020908@delphij.net> <86r4sqasrt.fsf@kopusha.home.net> <672D93D3-D4B1-432E-AE53-98E6C05B8BE4@lists.zabbadoz.net> <86zk7da10y.fsf@in138.ua3> <E909B0C0-F4DE-4110-B151-98FAC9330B82@lists.zabbadoz.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, 7 Jul 2012 20:38:23 +0000 Bjoern A. Zeeb wrote: BAZ> On 6. Jul 2012, at 05:53 , Mikolaj Golub wrote: >> >> On Thu, 5 Jul 2012 20:21:53 +0000 Bjoern A. Zeeb wrote: >> >> BAZ> On 5. Jul 2012, at 19:53 , Mikolaj Golub wrote: >> >>>> >>>> On Thu, 05 Jul 2012 12:18:20 -0700 Xin Li wrote: >>>> >>>> XL> Hi, Mikolaj, >>>> >>>> XL> On 07/04/12 00:00, Mikolaj Golub wrote: >>>>>> Is this observed after destroying epair? There is an issue with >>>>>> epair: on destroy, when epair_clone_destroy() calls >>>>>> ether_ifdetach() for its second half it does not switch to its vnet >>>>>> and if_detach_internal() can't find the interface and just returns. >>>>>> As a result V_ifnet list is left with dead reference. >>>> >>>> XL> Yes. >>>> >>>>>> http://lists.freebsd.org/pipermail/freebsd-virtualization/2011-January/000628.html >>>>>> >>>>>> Here is an updated patch against CURRENT: >>>>>> >>>>>> http://people.freebsd.org/~trociny/if_epair.c.epair_clone_destroy.1.patch >>>> >>>> XL> Your >>>>>> >>>> XL> patch did fixed the problem, thanks! Are you going to commit it >>>> XL> against -HEAD and then MFC after a while? >>>> >>>> I would like Bjoern review it before me committing, or at least tell he does >>>> not mind, if he does not have time to review -) >> >> BAZ> To me the patch looks wrong; I am wondering if someone broke some other central >> BAZ> assumptions but given I cannot currently spend time on this and if it fixes things >> BAZ> feel free to go ahead. >> >> If you told what looks wrong I could try to dig at that direction and might be >> back with a better solution, instead of committing a presumably wrong fix. BAZ> sorry; vnet.c:vnet_destroy() should dtrt already wrt to interfaces moving to parents BAZ> and being detached. But this is not the issue with vnet_destroy() not moving interfaces to parents. It does move them. It is with destroying epair. When epair that has its ends in different vnets is destroyed, and ether_ifdetach() is called for the second end without switching to its vnet it fails to find the iface in the wrong vnet and just silently returns (which I think is also wrong: if_detach_internal() should panic here). As a result the pointer is not removed from vnet ifnet list. And later, when someone is traversing this list and tries to access the pointer (this is often vnet_destroy(), which is usually called after removing interfaces, but might be e.g. ifconfig) dead pointer dereference occurs. My patch just makes epair_clone_destroy() switch to the proper vnet before calling ether_ifdetach(). Or have I missed your point? -- Mikolaj Golub
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?86obnqq94x.fsf>