Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 28 Jun 2013 03:03:46 +0200
From:      Mateusz Guzik <mjguzik@gmail.com>
To:        Mikolaj Golub <trociny@FreeBSD.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r252313 - head/sys/kern
Message-ID:  <20130628010345.GA25051@dft-labs.eu>
In-Reply-To: <201306271914.r5RJE4on047806@svn.freebsd.org>
References:  <201306271914.r5RJE4on047806@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
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.

-- 
Mateusz Guzik <mjguzik gmail.com>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130628010345.GA25051>