From owner-freebsd-arch@FreeBSD.ORG Fri Dec 17 20:59:25 2004 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id D872D16A4CE; Fri, 17 Dec 2004 20:59:25 +0000 (GMT) Received: from moutng.kundenserver.de (moutng.kundenserver.de [212.227.126.190]) by mx1.FreeBSD.org (Postfix) with ESMTP id 632ED43D1D; Fri, 17 Dec 2004 20:59:25 +0000 (GMT) (envelope-from max@love2party.net) Received: from [212.227.126.209] (helo=mrelayng.kundenserver.de) by moutng.kundenserver.de with esmtp (Exim 3.35 #1) id 1CfPC8-0007zP-00; Fri, 17 Dec 2004 21:59:24 +0100 Received: from [217.227.152.17] (helo=donor.laier.local) by mrelayng.kundenserver.de with asmtp (TLSv1:RC4-MD5:128) (Exim 3.35 #1) id 1CfPC7-0004cA-00; Fri, 17 Dec 2004 21:59:24 +0100 From: Max Laier To: freebsd-arch@freebsd.org Date: Fri, 17 Dec 2004 21:59:15 +0100 User-Agent: KMail/1.7.1 References: <94AE3F5A-4CD6-11D9-8BD6-000A95C4D7BC@FreeBSD.org> <200412131232.55051.max@love2party.net> <21F934EB-4FE7-11D9-B1F7-000A95C4D7BC@FreeBSD.org> In-Reply-To: <21F934EB-4FE7-11D9-B1F7-000A95C4D7BC@FreeBSD.org> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="nextPart12125019.tk7FQXOsAd"; protocol="application/pgp-signature"; micalg=pgp-sha1 Content-Transfer-Encoding: 7bit Message-Id: <200412172159.22871.max@love2party.net> X-Provags-ID: kundenserver.de abuse@kundenserver.de auth:61c499deaeeba3ba5be80f48ecc83056 cc: freebsd-current@freebsd.org cc: Suleiman Souhlal Subject: Re: sysctl locking X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 17 Dec 2004 20:59:26 -0000 --nextPart12125019.tk7FQXOsAd Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline On Friday 17 December 2004 05:50, Suleiman Souhlal wrote: > Hello, > > On Dec 13, 2004, at 6:32 AM, Max Laier wrote: > > 1) Extend sysctl_add_oid() to accept an additional mutex argument. > > 2) Extend the simple sysctl handler to use this mutex to protect the > > actual > > write(?read?). We must not hold the mutex during the useland copy > > in/out so > > we must move to temporary storage. > > 3) To maintain the current API and behavior we use &Giant as the > > default > > fallback argument. This might need some extension for complex > > handler (i.e. > > no mutex given -> acquire Giant before calling the complex handler). > > > > What do people think of this? Does it make any sense? Should we be > > concerned > > at all? Does the extension make sense? Comments? > > I have implemented this. The diff is at > http://people.freebsd.org/~ssouhlal/sysctl-sx-locking-20041214.diff It still looks like you call oid_handler() without grabbing the lock. You=20 should do this at least for the "oid_mtx =3D=3D &Giant"-case and - in my op= inion=20 =2D it should be done for the default case as well, so that oid_handler() c= an=20 assert the mutex. > It also needs the patch at > http://people.freebsd.org/~ssouhlal/sx_xlocked.diff which introduces a > sx_xlocked() function. Just spotted an error: @@ -884,10 +1015,14 @@ outlen =3D strlen((char *)arg1)+1; tmparg =3D malloc(outlen, M_SYSCTLTMP, M_WAITOK); =20 + if (oidp->oid_mtx) + mtx_lock(oidp->oid_mtx); if (strlcpy(tmparg, (char *)arg1, outlen) >=3D outlen) { free(tmparg, M_SYSCTLTMP); goto retry; <--- this will break the lock order, no? } + if (oidp->oid_mtx) + mtx_unlock(oidp->oid_mtx); =20 error =3D SYSCTL_OUT(req, tmparg, outlen); free(tmparg, M_SYSCTLTMP); > Unfortunately, we still need to look at every single SYSCTL_PROC, and > make either grab Giant, lock correctly, or make sure it doesn't need > any locking. :( =2D-=20 /"\ Best regards, | mlaier@freebsd.org \ / Max Laier | ICQ #67774661 X http://pf4freebsd.love2party.net/ | mlaier@EFnet / \ ASCII Ribbon Campaign | Against HTML Mail and News --nextPart12125019.tk7FQXOsAd Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.2.6 (FreeBSD) iD8DBQBBw0iqXyyEoT62BG0RArpeAJ9jrVuSwJYl6GqiYfEVaoVty34u/ACeOadk 0rIxAblyICRqPlD8cxn/6IQ= =S50c -----END PGP SIGNATURE----- --nextPart12125019.tk7FQXOsAd--