From owner-freebsd-current  Fri Aug 25 18:56:51 1995
Return-Path: current-owner
Received: (from majordom@localhost)
          by freefall.FreeBSD.org (8.6.11/8.6.6) id SAA13376
          for current-outgoing; Fri, 25 Aug 1995 18:56:51 -0700
Received: from cs.weber.edu (cs.weber.edu [137.190.16.16])
          by freefall.FreeBSD.org (8.6.11/8.6.6) with SMTP id SAA13370
          for <current@freebsd.org>; Fri, 25 Aug 1995 18:56:50 -0700
Received: by cs.weber.edu (4.1/SMI-4.1.1)
	id AA13784; Fri, 25 Aug 95 19:58:30 MDT
From: terry@cs.weber.edu (Terry Lambert)
Message-Id: <9508260158.AA13784@cs.weber.edu>
Subject: Re: Re(4): KERNEL PATCHES FOR NFS LOCKING SUPPORT (fwd)
To: current@freebsd.org
Date: Fri, 25 Aug 95 19:58:29 MDT
X-Mailer: ELM [version 2.4dev PL52]
Sender: current-owner@freebsd.org
Precedence: bulk


Andrew, who's working on the hard part of NFS locking, has pointed out
some errors in my NFS locking patches.  Here are the patches to correct
the errors.  They assume the previous patches have been applied.  The
comments or the if statements were correct, depending on your point
of view.

----

Rats.  I switched the copyin's -- you missed the "suser" returns 0 or
EPERM -- 0 if it's root.  And I missed 2 value compares and 2 copyin and
1 copyout swap.

The point was for a non-root call or for a call with the non "remote"
versions to use the old structure for the copyin/copyout for binary
compatability with old root and non-root code, and to protect against
proxy calls from non-root users.  I knew I was going to out-clever
myself here... I combined the cases at the last minute, and didn't
save before generating the patch.  8-(.


Here is a patch that fixes the oversights (3 of them!):
==============================================================================
*** kern_descrip.c.BAD	Fri Aug 25 18:38:20 1995
--- kern_descrip.c	Fri Aug 25 18:45:51 1995
***************
*** 263,274 ****
  		vp = (struct vnode *)fp->f_data;
  		if( suser(p->p_ucred, &p->p_acflag) || ( uap->cmd == F_SETLKW) || ( uap->cmd == F_SETLK)) {
  			/* Copy in the lock structure */
- 			error = copyin((caddr_t)uap->arg, (caddr_t)&flk, sizeof(flk));
- 		} else {	/* non-root/non-remote*/
- 			/* Copy in the lock structure */
  			error = copyin((caddr_t)uap->arg, (caddr_t)&flk, sizeof(struct oflock));
  			flk.l_rsys = FLOCK_LOCAL_LOCK;
  			flk.l_rpid = FLOCK_LOCAL_LOCK;
  		}
  		if (error)
  			return (error);
--- 263,274 ----
  		vp = (struct vnode *)fp->f_data;
  		if( suser(p->p_ucred, &p->p_acflag) || ( uap->cmd == F_SETLKW) || ( uap->cmd == F_SETLK)) {
  			/* Copy in the lock structure */
  			error = copyin((caddr_t)uap->arg, (caddr_t)&flk, sizeof(struct oflock));
  			flk.l_rsys = FLOCK_LOCAL_LOCK;
  			flk.l_rpid = FLOCK_LOCAL_LOCK;
+ 		} else {	/* root & remote*/
+ 			/* Copy in the lock structure */
+ 			error = copyin((caddr_t)uap->arg, (caddr_t)&flk, sizeof(flk));
  		}
  		if (error)
  			return (error);
***************
*** 301,314 ****
  		if (fp->f_type != DTYPE_VNODE)
  			return (EBADF);
  		vp = (struct vnode *)fp->f_data;
! 		if( suser(p->p_ucred, &p->p_acflag)) {
! 			/* Copy in the lock structure */
! 			error = copyin((caddr_t)uap->arg, (caddr_t)&flk, sizeof (flk));
! 		} else {	/* non-root/non-remote*/
  			/* Copy in the lock structure */
  			error = copyin((caddr_t)uap->arg, (caddr_t)&flk, sizeof(struct oflock));
  			flk.l_rsys = FLOCK_LOCAL_LOCK;
  			flk.l_rpid = FLOCK_LOCAL_LOCK;
  		}
  		if (error)
  			return (error);
--- 301,314 ----
  		if (fp->f_type != DTYPE_VNODE)
  			return (EBADF);
  		vp = (struct vnode *)fp->f_data;
! 		if( suser(p->p_ucred, &p->p_acflag) || ( uap->cmd == F_GETLK)) {
  			/* Copy in the lock structure */
  			error = copyin((caddr_t)uap->arg, (caddr_t)&flk, sizeof(struct oflock));
  			flk.l_rsys = FLOCK_LOCAL_LOCK;
  			flk.l_rpid = FLOCK_LOCAL_LOCK;
+ 		} else {	/* root & remote*/
+ 			/* Copy in the lock structure */
+ 			error = copyin((caddr_t)uap->arg, (caddr_t)&flk, sizeof (flk));
  		}
  		if (error)
  			return (error);
***************
*** 316,325 ****
  			flk.l_start += fp->f_offset;
  		if ((error = VOP_ADVLOCK(vp,(caddr_t)p,F_GETLK,&flk,F_POSIX)))
  			return (error);
! 		if( suser(p->p_ucred, &p->p_acflag)) {
! 			return (copyout((caddr_t)&flk, (caddr_t)uap->arg, sizeof(flk)));
! 		} else {	/* non-root/local*/
  			return (copyout((caddr_t)&flk, (caddr_t)uap->arg, sizeof(struct oflock)));
  		}
  
  	default:
--- 316,325 ----
  			flk.l_start += fp->f_offset;
  		if ((error = VOP_ADVLOCK(vp,(caddr_t)p,F_GETLK,&flk,F_POSIX)))
  			return (error);
! 		if( suser(p->p_ucred, &p->p_acflag) || ( uap->cmd == F_GETLK)) {
  			return (copyout((caddr_t)&flk, (caddr_t)uap->arg, sizeof(struct oflock)));
+ 		} else {	/* root & remote*/
+ 			return (copyout((caddr_t)&flk, (caddr_t)uap->arg, sizeof(flk)));
  		}
  
  	default:
==============================================================================


					Terry Lambert
					terry@cs.weber.edu
---
Any opinions in this posting are my own and not those of my present
or previous employers.