Re: kern/41489: setpriority(2) returns EACCES instead of EPERM

[Elad Efrat <elad%NetBSD.org@localhost>]

The following reply was made to PR kern/41489; it has been noted by GNATS.

From: Elad Efrat <elad%NetBSD.org@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: 
Subject: Re: kern/41489: setpriority(2) returns EACCES instead of EPERM
Date: Tue, 26 May 2009 02:26:50 +0300

 Christos Zoulas wrote:
 > The following reply was made to PR kern/41489; it has been noted by GNATS.
 > 
 > From: christos%zoulas.com@localhost (Christos Zoulas)
 > To: gnats-bugs%NetBSD.org@localhost, kern-bug-people%netbsd.org@localhost, 
 >      gnats-admin%netbsd.org@localhost, netbsd-bugs%netbsd.org@localhost, 
 > ekamperi%gmail.com@localhost
 > Cc: 
 > Subject: Re: kern/41489: setpriority(2) returns EACCES instead of EPERM
 > Date: Mon, 25 May 2009 18:38:09 -0400
 > 
 >  On May 25, 10:25pm, elad%NetBSD.org@localhost (Elad Efrat) wrote:
 >  -- Subject: Re: kern/41489: setpriority(2) returns EACCES instead of EPERM
 >  
 >  |  My bad: I was looking at the wrong part of the code (specifically the 
 >  |  EACCES at the bottom rather than the EPERM at the top).
 >  |  
 >  |  Anyway, the fix here isn't so obvious; specifically, the original check 
 >  |  checked both the effective and the real uid ("root" is a user with 
 >  |  effective uid 0). Additionally, the documentation (not ours) doesn't 
 >  |  necessarily specify a super-user, but rather a user with the proper 
 >  |  privileges, which is more correct. We have to decide if we want to 
 >  |  maintain the behavior (uid or euid 0 -> no EPERM, which is IMHO wrong), 
 >  |  fix it (euid 0 -> no EPERM, IMHO right, can simply be a 
 >  |  KAUTH_GENERIC_ISSUSER for now), or do something completely different 
 >  |  (like make listeners return errno values and weigh them, similar to 
 >  |  FreeBSD, long-term goal).
 >  |  
 >  |  The attached diff is simply restores the original checks.
 >  |  
 >  |  -e.
 >  
 >  Can't this be abstracted to a KAUTH_CHANGE_RESOURCE call or at least
 >  we should cache the uid and gid variables.
 
 It can and it will be, only that IIUC we want something that can be
 easily pulled up to netbsd-5.
 
 The issue here is a bit bigger than just this. When I did the suser
 secmodel, I made a mistake and moved some logic into it from the kernel,
 namely uid matching. Now that I think of it, we should have that logic
 as a "default" routine in the kernel relevant to the subsystem, and the
 suser secmodel should only check if the user is root or not (similar to
 how securelevel only checks the securelevel). This touches other aspects
 that I'd like to revisit as well; an rlimit interface (rather than open
 coded checks), for example.
 
 Fixing it, presuming we go with what I suggest (which applies to other
 parts of the code) will require a bit more changes than just introducing
 an action/request, and I'd like to have them properly brought up for
 review rather than decided in a PR's audit trail...
 
 What I suggest for now is going forward with putting back the original
 test to fix the issue in HEAD and netbsd-5, and I (or anyone, for that 
 matter) will take a look at a better solution when I (they) get the 
 time. On the other hand, since code can be changed, I will obviously
 not object to any other solution. :)
 
 -e.