From owner-freebsd-current@FreeBSD.ORG Tue Sep 23 17:34:41 2008 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id A7E431065673; Tue, 23 Sep 2008 17:34:41 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from mail.terabit.net.ua (mail.terabit.net.ua [195.137.202.147]) by mx1.freebsd.org (Postfix) with ESMTP id 427928FC1B; Tue, 23 Sep 2008 17:34:41 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from skuns.zoral.com.ua ([91.193.166.194] helo=mail.zoral.com.ua) by mail.terabit.net.ua with esmtp (Exim 4.63 (FreeBSD)) (envelope-from ) id 1KiBmZ-000BlK-I6; Tue, 23 Sep 2008 20:34:39 +0300 Received: from deviant.kiev.zoral.com.ua (root@deviant.kiev.zoral.com.ua [10.1.1.148]) by mail.zoral.com.ua (8.14.2/8.14.2) with ESMTP id m8NHYZvN090463 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 23 Sep 2008 20:34:36 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: from deviant.kiev.zoral.com.ua (kostik@localhost [127.0.0.1]) by deviant.kiev.zoral.com.ua (8.14.2/8.14.2) with ESMTP id m8NHYZoV022302; Tue, 23 Sep 2008 20:34:35 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by deviant.kiev.zoral.com.ua (8.14.3/8.14.3/Submit) id m8NHYZw2022301; Tue, 23 Sep 2008 20:34:35 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: deviant.kiev.zoral.com.ua: kostik set sender to kostikbel@gmail.com using -f Date: Tue, 23 Sep 2008 20:34:35 +0300 From: Kostik Belousov To: Maksim Yevmenkin Message-ID: <20080923173435.GW47828@deviant.kiev.zoral.com.ua> References: <48D2F942.4070801@FreeBSD.org> <20080919084201.GD44330@wep4035.physik.uni-wuerzburg.de> <48D38DFF.8000803@FreeBSD.org> <20080919203310.GA34131@localhost.my.domain> <48D8196E.7020005@FreeBSD.org> <20080923094134.GM47828@deviant.kiev.zoral.com.ua> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="Zd8I2GZVcdxtyaG/" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.2.3i X-Virus-Scanned: ClamAV version 0.93.3, clamav-milter version 0.93.3 on skuns.kiev.zoral.com.ua X-Virus-Status: Clean X-Spam-Status: No, score=-4.4 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on skuns.kiev.zoral.com.ua X-Virus-Scanned: mail.terabit.net.ua 1KiBmZ-000BlK-I6 79293053081245a315d5ec124da90ce6 X-Terabit: YES Cc: Alexey Shuvaev , freebsd-current@freebsd.org, Ed Schouten Subject: Re: Interface auto-cloning bug or feature? X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 23 Sep 2008 17:34:41 -0000 --Zd8I2GZVcdxtyaG/ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Sep 23, 2008 at 10:19:13AM -0700, Maksim Yevmenkin wrote: > On 9/23/08, Kostik Belousov wrote: >=20 > [...] >=20 > > > attached is a slightly better patch for tap(4). the idea is to use > > > extra ALLOCATED flag that prevents the race Kostik pointed out. could > > > you please give it a try? any review comments are greatly appreciate= d. > > > if this is acceptable, i will prepare something similar for tun(4) > > > > The tap should use make_dev_credf(MAKEDEV_REF) instead of > > make_dev/dev_ref sequence in the clone handler. For similar reasons, I > > think it is slightly better to do a dev_ref() immediately after setting > > the TAP_ALLOCATED flag without dropping tapmtx. >=20 > could you please explain why it is better? >=20 > > I cannot figure out how tap_clone_create/tap_clone_destroy are being > > called. Can it be garbage-collected ? >=20 > ah, this is interface clone feature, i.e. one can do 'ifconfig tap0 > create/destroy' to create an interface and device node. take a look at > IFC_SIMPLE_DECLARE() macro. Thanks for the explanation. >=20 > > The whole module unload sequence looks unsafe. >=20 > yes, it is unsafe. it even has comment about it :) i guess, i could > fix it too while i'm at it :) One of the reason why the module unload is unsafe is the complete lack of synchronization between cloner and device destruction. Leaving tapmtx and tp->tap_mtx protected region in the clone handler, you allow for module unload routine to destroy device, and then dev_ref() would operate on the freed memory. Not that doing that without dropping the mutex(es) fix the bug, but at least it is a right move, it seems. At least this would trade a crash to a memory leak. --Zd8I2GZVcdxtyaG/ Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (FreeBSD) iEYEARECAAYFAkjZKKsACgkQC3+MBN1Mb4gikACcD4dbxfiairgFVR4bN1UcQW98 cNsAoO5vdmvsSU0omGGvfpCpELiw4ses =vSMA -----END PGP SIGNATURE----- --Zd8I2GZVcdxtyaG/--