Prev: [PATCH] Input: mousedev - Fix mousedev_poll() to advertise when /dev/input/mice is writable.
Next: xfs: track AGs with reclaimable inodes in per-ag radix tree
From: John Johansen on 15 Jul 2010 14:10
On 07/15/2010 10:27 AM, Serge E. Hallyn wrote:
> Quoting John Johansen (john.johansen(a)canonical.com):
>> AppArmor hooks to interface with the LSM, module parameters and module
>> initialization.
>>
>> Signed-off-by: John Johansen <john.johansen(a)canonical.com>
>
> Thanks, John - looks good overall. Comments:
>
> ...
>
>> +static int apparmor_ptrace_access_check(struct task_struct *child,
>> + unsigned int mode)
>> +{
>> + int rc;
>> +
>> + rc = cap_ptrace_access_check(child, mode);
>> + if (rc)
>> + return rc;
>> +
>> + return aa_ptrace(current, child, mode);
>> +}
>> +
>> +static int apparmor_ptrace_traceme(struct task_struct *parent)
>> +{
>
> Just curious - why aren't you calling cap_ptrace_traceme() first here?
>
err, we should be. I'm not sure where that got dropped. I'll go through
and re audit all of these.

thanks
>> + return aa_ptrace(parent, current, PTRACE_MODE_ATTACH);
>> +}
>> +
>> +/* Derived from security/commoncap.c:cap_capget */
>> +static int apparmor_capget(struct task_struct *target, kernel_cap_t *effective,
>> + kernel_cap_t *inheritable, kernel_cap_t *permitted)
>> +{
>> + struct aa_profile *profile;
>> + const struct cred *cred;
>> +
>> + rcu_read_lock();
>> + cred = __task_cred(target);
>> + profile = aa_cred_profile(cred);
>> +
>> + *effective = cred->cap_effective;
>> + *inheritable = cred->cap_inheritable;
>> + *permitted = cred->cap_permitted;
>> +
>> + if (!unconfined(profile))
>> + *effective = cap_intersect(*effective, profile->caps.allow);
>
> Should you mask permitted too? Otherwise you might confuse a userspace
> lib which assumes it's caller previously culled pE, and that it can
> nwo refill it from pP.
>
yes indeed thanks

>> + rcu_read_unlock();
>> +
>> + return 0;
>> +}
>> +
>> +static int apparmor_capable(struct task_struct *task, const struct cred *cred,
>> + int cap, int audit)
>> +{
>> + struct aa_profile *profile;
>> + /* cap_capable returns 0 on success, else -EPERM */
>> + int error = cap_capable(task, cred, cap, audit);
>
> jinkeys, it might be just me, but i'd have spend 2 mins less looking
> at this if you'd done
>
> if (error)
> return error;
>
> here, simplifying the condition below.
>
>> +
>> + profile = aa_cred_profile(cred);
>> + if (!error && !unconfined(profile))
>> + error = aa_capable(task, profile, cap, audit);
>> +
>> + return error;
>> +}
>
yeah, that is better

thanks Serge
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 | 
Pages: 1
Prev: [PATCH] Input: mousedev - Fix mousedev_poll() to advertise when /dev/input/mice is writable.
Next: xfs: track AGs with reclaimable inodes in per-ag radix tree