On 10/24/2012 6:25 AM, Tetsuo Handa wrote:
> Kees Cook wrote:
>> prctl control needs special handling. When an option is unhandled,
>> it'll return -ENOSYS. Also, some prctls return non-zero results, so
>> they could mask each other. I think it would make sense to walk the
>> composer list as long as the return value is -ENOSYS. As soon as it
>> isn't, then stop the call chain and return the value. Ultimately, this
>> means that the LSM order defines who get access to a given prctl
>> option (though with caps last).
I don't see why you think prctl is a special case. For starters,
only Yama does anything with it, and Yama does the expected
"call cap if you care" thingy. Yes, the -ENOSYS return is there,
but the current implementation will handle it correctly because
it's in cap_task_prctl.
What am I missing?
>>
>> int i, rc, called = 0;
>> for (i = 1; i < lsm_count; i++) {
>> if (!composer_ops[i]->task_prctl)
>> continue;
>> rc = composer_ops[i]->task_prctl(option, arg2, arg3, arg4, arg5);
>> called = 1;
>> if (rc != -ENOSYS)
>> return rc;
>> }
>> if (!called and composer_ops[0]->task_prctl)
>> return composer_ops[0]->task_prctl(option, arg2, arg3, arg4, arg5);
>> return rc;
> With optimization, security_task_prctl() would look like below? I removed
> "int called" usage so that LSM modules which implement ->task_prctl need not to
> call cap_task_prctl(). Also, I assume "for (i = 1; i < lsm_count; i++)" will be
> changed to "for (i = 0; i < lsm_count; i++)" if this optimization is accepted.
>
> int security_task_prctl(int option, unsigned long arg2, unsigned long arg3,
> unsigned long arg4, unsigned long arg5)
> {
> int i;
> for (i = 1; i < lsm_count; i++) {
> int rc;
> if (!composer_ops[i]->task_prctl)
> continue;
> rc = composer_ops[i]->task_prctl(option, arg2, arg3, arg4, arg5);
> if (rc != -ENOSYS)
> return rc;
> }
> return cap_task_prctl(option, arg2, arg3, arg4, arg5);
> }
>
> Same thing applies to security_bprm_set_creds() etc. so that LSM modules which
> implement ->bprm_set_creds need not to call cap_bprm_set_creds()?
>
> int security_bprm_set_creds(struct linux_binprm *bprm)
> {
> int i;
> for (i = 1; i < lsm_count; i++) {
> int rc;
> if (!composer_ops[i]->bprm_set_creds)
> continue;
> rc = composer_ops[i]->bprm_set_creds(bprm);
> if (rc)
> return rc;
> }
> return cap_bprm_set_creds(bprm);
> }
>
> Casey Schaufler wrote:
>> I still think that we want to "call all" rather than "bail on fail".
> If prctl() is "bail out upon handled" and security_bprm_set_creds() is
> "bail out upon fail", wouldn't it look natural to do like below?
>
> int security_inode_permission(struct inode *inode, int mask)
> {
> if (unlikely(IS_PRIVATE(inode)))
> return 0;
> for (i = 1; i < lsm_count; i++) {
> int rc;
> if (!composer_ops[i]->inode_permission)
> continue;
> rc = composer_ops[i]->inode_permission(inode, mask);
> if (rc)
> return rc;
> }
> return 0;
> }
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@xxxxxxxxxxxxx with
the words "unsubscribe selinux" without quotes as the message.
