svn commit: r273441 - in head/sys: kern sys

[Konstantin Belousov]

On Wed, Oct 22, 2014 at 12:23:44AM +0000, Mateusz Guzik wrote:
> Author: mjg
> Date: Wed Oct 22 00:23:43 2014
> New Revision: 273441
> URL: https://svnweb.freebsd.org/changeset/base/273441
> 
> Log:
>   filedesc: cleanup setugidsafety a little
>   
>   Rename it to fdsetugidsafety for consistency with other functions.
>   
>   There is no need to take filedesc lock if not closing any files.
>   
>   The loop has to verify each file and we are guaranteed fdtable has space
>   for at least 20 fds. As such there is no need to check fd_lastfile.
^^^^^^^^^^^^^^^^^^^^^^^^ *

>   
>   While here tidy up is_unsafe.
> 
> Modified:
>   head/sys/kern/kern_descrip.c
>   head/sys/kern/kern_exec.c
>   head/sys/sys/filedesc.h
> 
> Modified: head/sys/kern/kern_descrip.c
> ==============================================================================
> --- head/sys/kern/kern_descrip.c	Tue Oct 21 23:57:31 2014	(r273440)
> +++ head/sys/kern/kern_descrip.c	Wed Oct 22 00:23:43 2014	(r273441)
> @@ -2078,23 +2078,23 @@ fdescfree(struct thread *td)
>   * Since setugidsafety calls this only for fd 0, 1 and 2, this check is
>   * sufficient.  We also don't check for setugidness since we know we are.
>   */
> -static int
> +static bool
>  is_unsafe(struct file *fp)
>  {
> -	if (fp->f_type == DTYPE_VNODE) {
> -		struct vnode *vp = fp->f_vnode;
> +	struct vnode *vp;
>  
> -		if ((vp->v_vflag & VV_PROCDEP) != 0)
> -			return (1);
> -	}
> -	return (0);
> +	if (fp->f_type != DTYPE_VNODE)
> +		return (false);
> +
> +	vp = fp->f_vnode;
> +	return ((vp->v_vflag & VV_PROCDEP) != 0);
>  }
>  
>  /*
>   * Make this setguid thing safe, if at all possible.
>   */
>  void
> -setugidsafety(struct thread *td)
> +fdsetugidsafety(struct thread *td)
>  {
>  	struct filedesc *fdp;
>  	struct file *fp;
> @@ -2102,12 +2102,10 @@ setugidsafety(struct thread *td)
>  
>  	fdp = td->td_proc->p_fd;
>  	KASSERT(fdp->fd_refcnt == 1, ("the fdtable should not be shared"));
> -	FILEDESC_XLOCK(fdp);
> -	for (i = 0; i <= fdp->fd_lastfile; i++) {
> -		if (i > 2)
> -			break;
> +	for (i = 0; i <= 2; i++) {
[*] This requires an assert.

>  		fp = fdp->fd_ofiles[i].fde_file;
>  		if (fp != NULL && is_unsafe(fp)) {
> +			FILEDESC_XLOCK(fdp);
>  			knote_fdclose(td, i);
>  			/*
>  			 * NULL-out descriptor prior to close to avoid