svn commit: r252313 - head/sys/kern

[Konstantin Belousov]

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
> > 
> > Log:
> >   To avoid LOR, always drop the filedesc lock before exporting fd to sbuf.
> >   
> >   Reviewed by:	kib
> >   MFC after:	3 days
> > 
> > Modified:
> >   head/sys/kern/kern_descrip.c
> > 
> > Modified: head/sys/kern/kern_descrip.c
> > ==============================================================================
> > --- 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,  
> >  		 * re-validate and re-evaluate its properties when
> >  		 * the loop continues.
> >  		 */
> > -		if (type == KF_TYPE_VNODE || type == KF_TYPE_FIFO)
> > -			FILEDESC_SUNLOCK(fdp);
> > +		FILEDESC_SUNLOCK(fdp);
> >  		error = export_fd_to_sb(data, type, i, fflags, refcnt,
> >  		    offset, fd_cap_rights, kif, sb, &remainder);
> > -		if (type == KF_TYPE_VNODE || type == KF_TYPE_FIFO)
> > -			FILEDESC_SLOCK(fdp);
> > +		FILEDESC_SLOCK(fdp);
> >  		if (error)
> >  			break;
> >  	}
> 
> 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).
> 
> 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 834 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/svn-src-all/attachments/20130628/26d35f87/attachment.sig>