A question about ipcperm() call?

[Robert Watson]

On Mon, 24 Jul 2006, Xin LI wrote:

> On 7/24/06, Íû¾°Û¿ <shangjie.li at gmail.com> wrote:
>> The code for ipcperm() call :
>>  78 ipcperm(td, perm, mode)
>>  79         struct thread *td;
>>  80         struct ipc_perm *perm;
>>  81         int mode;
>>  82 {
>>  83         struct ucred *cred = td->td_ucred;
>>  84         int error;
>>  85
>>  86         if (cred->cr_uid != perm->cuid && cred->cr_uid != perm->uid) {
>>  87                 /*
>>  88                  * For a non-create/owner, we require privilege to
>>  89                  * modify the object protections.  Note: some other
>>  90                  * implementations permit IPC_M to be delegated to
>>  91                  * unprivileged non-creator/owner uids/gids.
>>  92                  */
>>  93                 if (mode & IPC_M) {
>>  94                         error = suser(td);
>>  95                         if (error)
>>  96                                 return (error);
>>  97                 }
>>  98                 /*
>>  99                  * Try to match against creator/owner group; if not, 
>> fall
>> 100                  * back on other.
>> 101                  */
>> 102                 mode >>= 3;
>> 103                 if (!groupmember(perm->gid, cred) &&
>> 104                     !groupmember(perm->cgid, cred))
>> 105                         mode >>= 3;
>> 106         } else {
>> 107                 /*
>> 108                  * Always permit the creator/owner to update the object
>> 109                  * protections regardless of whether the object mode
>> 110                  * permits it.
>> 111                  */
>> 112                 if (mode & IPC_M)
>> 113                         return (0);
>> 114         }
>> 115
>> 116         if ((mode & perm->mode) != mode) {
>> 117                 if (suser(td) != 0)
>> 118                         return (EACCES);
>> 119         }
>> 120         return (0);
>> 121 }
>> 
>> why not directly return the error in line 94?
>
> I think it makes sense to remove the assignment and the 'error' variable. 
> Let's see Robert's opinion.

In almost all cases, we return the error returned by suser() in order to 
encapsulate the notion that the "lack of privilege" error (EPERM) in suser() 
rather than scattered around.  I'm not sure it makes much difference, since 
functionally you will get back EPERM either way, but generally the convention 
has been for pieces of access control logic to return an error reflecting the 
nature of the success or failure (0, EPERM, EACCES, ESRCH, etc), and for the 
calling code not to perform any translation of the error unless strictly 
necessary.  For example, in procfs we do translate ESRCH into ENOENT, but only 
because ESRCH isn't a standard file system error, and ENOENT is the logical 
equivilent.  In our TrustedBSD SEBSD branch, the MAC Framework permits 
policies to modify the notion of privilege, so in principle they can return 
different errors from the privilege check, although I am not entirely sure 
they shold.  I am not sure that I agree that we should replace the current 
error assignment, as it will encode EPERM as the privilege error in ipcperm(), 
and prevent suser() from returning a different error.

Robert N M Watson
Computer Laboratory
University of Cambridge