Subject: Re: kern/35226: Problems with permissions in /usr/pkg/emul/linux/proc
To: None <kern-bug-people@netbsd.org, gnats-admin@netbsd.org,>
From: Elad Efrat <elad@NetBSD.org>
List: netbsd-bugs
Date: 12/24/2006 19:25:02
The following reply was made to PR kern/35226; it has been noted by GNATS.

From: Elad Efrat <elad@NetBSD.org>
To: gnats-bugs@NetBSD.org
Cc: Christos Zoulas <christos@zoulas.com>
Subject: Re: kern/35226: Problems with permissions in /usr/pkg/emul/linux/proc
 .
Date: Sun, 24 Dec 2006 21:20:25 +0200

 This is a multi-part message in MIME format.
 --------------020302050400080301050205
 Content-Type: text/plain; charset=ISO-8859-1
 Content-Transfer-Encoding: 7bit
 
 Christos Zoulas wrote:
 
 >  Well, the code before was:
 >  
 >  
 >          /*
 >           * Do not allow init to be modified while in secure mode; it
 >           * could be duped into changing the security level.
 >           */
 >          if (uio->uio_rw == UIO_WRITE && p == initproc && securelevel > -1)
 >                  return EPERM;
 >  
 >  
 >  Which always allowed reads and forbad writes for init when securelevel > -1
 >  This is not what the new code is doing is it, although the comment has remained
 >  the same? I did not look at the procfs authorize code, but if it is doing
 >  what the above if statement is doing, you should not need the case statement.
 
 right. we should probably do something like the attached diff. I think
 it'd allow us to leave the kauth(9) call where it is, move the kauth(9)
 call in procfs_open() outside the special case, and we'll get original
 functionality.
 
 please review.
 
 -e.
 
 --------------020302050400080301050205
 Content-Type: text/plain;
  name="pr35226.diff"
 Content-Transfer-Encoding: 7bit
 Content-Disposition: inline;
  filename="pr35226.diff"
 
 Index: secmodel_bsd44_suser.c
 ===================================================================
 RCS file: /usr/cvs/src/sys/secmodel/bsd44/secmodel_bsd44_suser.c,v
 retrieving revision 1.20
 diff -u -p -r1.20 secmodel_bsd44_suser.c
 --- secmodel_bsd44_suser.c	22 Dec 2006 11:13:22 -0000	1.20
 +++ secmodel_bsd44_suser.c	23 Dec 2006 18:01:35 -0000
 @@ -270,11 +270,39 @@ secmodel_bsd44_suser_process_cb(kauth_cr
  		result = KAUTH_RESULT_DENY;
  		break;
  
 -	case KAUTH_PROCESS_CANPROCFS:
 -		if (((u_long)arg1 == KAUTH_REQ_PROCESS_CANPROCFS_CTL) &&
 -		    !isroot)
 +	case KAUTH_PROCESS_CANPROCFS: {
 +		enum kauth_process_req req = (enum kauth_process_req)arg2;
 +		struct pfsnode *pfs = arg1;
 +
 +		if (isroot) {
 +			result = KAUTH_RESULT_ALLOW;
 +			break;
 +		}
 +
 +		if (req == KAUTH_REQ_PROCESS_CANPROCFS_CTL) {
 +			result = KAUTH_RESULT_DENY;
 +			break;
 +		}
 +
 +		switch (pfs->pfs_type) {
 +		case PFSregs:
 +		case PFSfpregs:
 +		case PFSmem:
 +			if (kauth_cred_getuid(cred) !=
 +			    kauth_cred_getuid(p->p_cred) ||
 +			    ISSET(p->p_flag, P_SUGID)) {
 +				result = KAUTH_RESULT_DENY;
 +				break;
 +			}
 +			/*FALLTHROUGH*/
 +		default:
 +			result = KAUTH_RESULT_ALLOW;
  			break;
 -		/*FALLTHROUGH*/
 +		}
 +
 +		break;
 +		}
 +
  	case KAUTH_PROCESS_CANPTRACE:
  	case KAUTH_PROCESS_CANSYSTRACE:
  		if (isroot) {
 
 --------------020302050400080301050205--