From owner-freebsd-virtualization@FreeBSD.ORG Sun Jul 8 20:47:54 2012 Return-Path: Delivered-To: freebsd-virtualization@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 7C2CC106564A for ; Sun, 8 Jul 2012 20:47:54 +0000 (UTC) (envelope-from to.my.trociny@gmail.com) Received: from mail-wg0-f50.google.com (mail-wg0-f50.google.com [74.125.82.50]) by mx1.freebsd.org (Postfix) with ESMTP id 076558FC12 for ; Sun, 8 Jul 2012 20:47:53 +0000 (UTC) Received: by wgbds11 with SMTP id ds11so10665138wgb.31 for ; Sun, 08 Jul 2012 13:47:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:references:x-comment-to:sender:date:in-reply-to :message-id:user-agent:mime-version:content-type; bh=jnzAR8B/wejl2sWI+BFhD0jMQ32nDZoLsjRC65BUeMA=; b=eoVzaEzE9ppnFbcZj9HBKWScuFMkROf1qf/7EcaX0G0ulmsFaqXK0qgFFsWnbKEdAi Nz2pRr4wB5fDJheQRPTGSD6uXXlDWBvafbmpHLEu3ttoiVNt48Hbc1MOLJBzMLooxlqu 9vx/66kfEoHVScy5850EtLQ/aaj70PbL7p/hDwK4uM121Yfeo8722Rz4X34n3iuiQODs ZNQ8UHsbCE8RGhGxu+PDSErSLvuLQRqM3Xot10bml/TlBmsDAVl9ecPvJ63a7WJC9fYn Cbzs0yemE1eKwionxchSYoLq6uVgmwjhy9yQ7ueeLdAUOsxbY6obm5pSrNSAFYcaNHhX esVw== Received: by 10.216.24.85 with SMTP id w63mr3717255wew.145.1341780034290; Sun, 08 Jul 2012 13:40:34 -0700 (PDT) Received: from localhost ([95.69.175.25]) by mx.google.com with ESMTPS id eu4sm19731426wib.2.2012.07.08.13.40.31 (version=TLSv1/SSLv3 cipher=OTHER); Sun, 08 Jul 2012 13:40:32 -0700 (PDT) From: Mikolaj Golub To: "Bjoern A. Zeeb" 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> X-Comment-To: Bjoern A. Zeeb Sender: Mikolaj Golub Date: Sun, 08 Jul 2012 23:40:30 +0300 In-Reply-To: (Bjoern A. Zeeb's message of "Sat, 7 Jul 2012 20:38:23 +0000") Message-ID: <86obnqq94x.fsf@kopusha.home.net> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.4 (berkeley-unix) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: d@delphij.net, FreeBSD virtualization mailing list Subject: Re: GPF when doing jail -r, possibly an use-after-free X-BeenThere: freebsd-virtualization@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Discussion of various virtualization techniques FreeBSD supports." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 08 Jul 2012 20:47:54 -0000 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