On 01/10/2011 05:51 PM, Serge Hallyn wrote: > Quoting Bastian Blank ([email protected]): >> On Mon, Jan 10, 2011 at 09:13:34PM +0000, Serge E. Hallyn wrote: >>> + const struct cred *cred = current_cred(); >>> + const struct cred *tcred = __task_cred(t); >>> + >>> + if (cred->user->user_ns != tcred->user->user_ns) { >>> + /* userids are not equivalent - either you have the >>> + capability to the target user ns or you don't */ >>> + if (ns_capable(tcred->user->user_ns, CAP_KILL)) >>> + return 1; >>> + return 0; >>> + } >>> + >>> + /* same user namespace - usual credentials checks apply */ >>> + if ((cred->euid ^ tcred->suid) && >>> + (cred->euid ^ tcred->uid) && >>> + (cred->uid ^ tcred->suid) && >>> + (cred->uid ^ tcred->uid) && >>> + !ns_capable(tcred->user->user_ns, CAP_KILL)) >>> + return 0; >>> + >>> + return 1; >> >> Isn't that equal to this? >> >> if (ns_capable(tcred->user->user_ns, CAP_KILL)) >> return 1; >> >> if (cred->user->user_ns == tcred->user->user_ns && >> (cred->euid == tcred->suid || >> cred->euid == tcred->uid || >> cred->uid == tcred->suid || >> cred->uid == tcred->uid)) >> return 1; >> >> return 0; >> >> I would consider this much easier to read. > > Unfortunately, it's actually not equivalent. when capable() > returns success, then it sets the current->flags |= PF_SUPERPRIV. > If permission is granted based on userids and the capability > isn't needed, then we don't want to needlessly set PF_SUPERPRIV.
A bit off-topic: does this means that c/r needs to save and restore this process flag ? > > That's why I'm going to such lengths to call capable() as a last > resort. IMHO, worth a one line comment in the code ... Oren. _______________________________________________ Containers mailing list [email protected] https://lists.linux-foundation.org/mailman/listinfo/containers _______________________________________________ Devel mailing list [email protected] https://openvz.org/mailman/listinfo/devel
