From owner-svn-src-head@FreeBSD.ORG Fri Jun 28 06:44:40 2013 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 64612187; Fri, 28 Jun 2013 06:44:40 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) by mx1.freebsd.org (Postfix) with ESMTP id B23BF1827; Fri, 28 Jun 2013 06:44:39 +0000 (UTC) Received: from tom.home (kostik@localhost [127.0.0.1]) by kib.kiev.ua (8.14.7/8.14.7) with ESMTP id r5S6iUKg024526; Fri, 28 Jun 2013 09:44:30 +0300 (EEST) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.8.3 kib.kiev.ua r5S6iUKg024526 Received: (from kostik@localhost) by tom.home (8.14.7/8.14.7/Submit) id r5S6iUlP024525; Fri, 28 Jun 2013 09:44:30 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Fri, 28 Jun 2013 09:44:30 +0300 From: Konstantin Belousov To: Mateusz Guzik Subject: Re: svn commit: r252313 - head/sys/kern Message-ID: <20130628064430.GK91021@kib.kiev.ua> References: <201306271914.r5RJE4on047806@svn.freebsd.org> <20130628010345.GA25051@dft-labs.eu> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="Qf+2chOW7Mplyx0V" Content-Disposition: inline In-Reply-To: <20130628010345.GA25051@dft-labs.eu> User-Agent: Mutt/1.5.21 (2010-09-15) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no version=3.3.2 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on tom.home Cc: Mikolaj Golub , svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 28 Jun 2013 06:44:40 -0000 --Qf+2chOW7Mplyx0V Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Jun 28, 2013 at 03:03:46AM +0200, Mateusz Guzik wrote: > On Thu, Jun 27, 2013 at 07:14:04PM +0000, Mikolaj Golub wrote: > > Author: trociny > > Date: Thu Jun 27 19:14:03 2013 > > New Revision: 252313 > > URL: http://svnweb.freebsd.org/changeset/base/252313 > >=20 > > Log: > > To avoid LOR, always drop the filedesc lock before exporting fd to sb= uf. > > =20 > > Reviewed by: kib > > MFC after: 3 days > >=20 > > Modified: > > head/sys/kern/kern_descrip.c > >=20 > > Modified: head/sys/kern/kern_descrip.c > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D > > --- head/sys/kern/kern_descrip.c Thu Jun 27 18:59:07 2013 (r252312) > > +++ head/sys/kern/kern_descrip.c Thu Jun 27 19:14:03 2013 (r252313) > > @@ -3427,12 +3427,10 @@ kern_proc_filedesc_out(struct proc *p, =20 > > * re-validate and re-evaluate its properties when > > * the loop continues. > > */ > > - if (type =3D=3D KF_TYPE_VNODE || type =3D=3D KF_TYPE_FIFO) > > - FILEDESC_SUNLOCK(fdp); > > + FILEDESC_SUNLOCK(fdp); > > error =3D export_fd_to_sb(data, type, i, fflags, refcnt, > > offset, fd_cap_rights, kif, sb, &remainder); > > - if (type =3D=3D KF_TYPE_VNODE || type =3D=3D KF_TYPE_FIFO) > > - FILEDESC_SLOCK(fdp); > > + FILEDESC_SLOCK(fdp); > > if (error) > > break; > > } >=20 > Is this really ok? What prevents given fd from going away during > export_fd_to_sb execution? Both DTYPE_VNODE and DTYPE_FIFO pass down > a vrefed vnode so these are safe. But for example DTYPE_SOCKET goes with > fp->f_data, which can go away in the meantime (or I'm misreading the code= ). >=20 > I suggest obtainng ref to fp and passing it down in all cases. Oops, I am sorry for missed this. But, I do not actually like the idea of referencing the fd. It de-facto means that the process calling the sysctl duped the descriptor, potentially causing the close to be performed in the sysctl context on dereference. Ideal solution would be to drop the filedesc lock between processing of the filedescriptors and draining sbuf while lock is dropped. --Qf+2chOW7Mplyx0V Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.20 (FreeBSD) iQIcBAEBAgAGBQJRzTDNAAoJEJDCuSvBvK1BuBQP/ifxuc80FOxwUDE5T/kCzvrk iG2TXKO/FAXbTh7RqUduHlQr8RPLJX1Pzp2gluOcyGLPmZ+IcgqesOsMskRukfFk GdMkEmYBqZihj9m5aDv9kYFf0G8igtOT2jfhpYF81QMcbO/4aE9Zz11TlN81ZXAw vikIkXNcErDkwjwPbQ5Lc59O7jz0HidVsJM1685M0R5dA68hDGjqaeTOebR1mTcQ 6hxJlp1vENVE2YnIkU0cgK54nQwdNe4v1jJMAqPIfIx3DgTBKXmBcERkcnrnO60N w3W2hLuQZEJQGLzdzIUyYlowVmWPrhf5PdvNEku5EUDXsCYfZhDIQRFR1RhFrnzS JK0QrkSJ5xMLSTFOEhPa2cum1sk6qE+6g/or+YIIq/Xb713cZYrkeLQAZWddSD+e 84P5mYLuLoVfjh5al4UvrEqMwD54YRNnX+pPsM2Dfpo50BMe/LlhGjEfaR8Qgumn vV44DMIA4vcHw9JkzSy4XH07WPYwg4GGBLUs1lo/0GR0bxryKiDqB/khazTul/PI ivSwlmaCFUOmYTlyS81PYcbC0UJPGRqqYJk82ElOnQqk5hsKsAkfbUwThr13R4iH ENTGGJuajdYfJRQZDQwIMs7D31h0tb9zmwrQWUO+mcLqzP4kjhcF1j/RTB8rfqfs PUMr5fMZGuHbZKwpK1K5 =AMMA -----END PGP SIGNATURE----- --Qf+2chOW7Mplyx0V--