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

[christos%zoulas.com@localhost (Christos Zoulas)]

On May 25, 11:30pm, elad%NetBSD.org@localhost (Elad Efrat) wrote:
-- Subject: Re: kern/41489: setpriority(2) returns EACCES instead of EPERM

| 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. :)

Sounds good to me...

christos