On 05/16/2013 03:38 PM, Seth Arnold wrote:
> On Wed, May 01, 2013 at 02:31:03PM -0700, John Johansen wrote:
>> The labeling of files is implied by the set of rules and profiles.
>> Add the ability to set implicit labels on files to reduce the number
>> of path and rule lookups that are needed.
> 
>> +static bool __aa_label_remove(struct aa_labelset *ls, struct aa_label 
>> *label);
>> +void aa_label_kref(struct kref *kref)
>> +{
>> +    struct aa_label *l = container_of(kref, struct aa_label, count);
>> +    struct aa_labelset *ls = labels_set(l);
>> +    unsigned long flags;
>> +
>> +
>> +    write_lock_irqsave(&ls->lock, flags);
>> +    (void) __aa_label_remove(ls, l);
>> +    write_unlock_irqrestore(&ls->lock, flags);
>> +
>> +    /* TODO: if compound label and not invalid add to reclaim cache */
>> +    call_rcu(&l->rcu, label_free_rcu);
>> +}
> 
> I know you said The Future would make this not matter, but
> __aa_label_remove() and the locking could be replaced with
> aa_label_remove() in this function.

yeah, fixed

> 
>> +    label->sid = aa_alloc_sid();
>> +    if (label->sid == AA_SID_INVALID)
>> +            return false;
> 
> Not really a problem here, but under what kind of load would we
> eventually wrap sids?
> 
secids (sids) are just nasty. I would love to kill them full stop. But until
we can get a proper security pointer pushed to all parts of the kernel we
are stuck with them.

So the current sid stuff is just a place holder we don't actually use them
yet.

Sids will wrap, just how fast will depend on how often profile loads are
happening and how often compound labels are being created. The sid is
a u32 so it will wrap. I just can't say when it is very policy dependent.

The design around sids is that when we start actually using them we need
to be able to reverse map them to their label, and to be able to recycle
them if they never get put on an object that requires them. If a sid
ever gets put on an object that requires them the mapping is fixed until
the kernel is rebooted, as we have no lifetime information on the sid.

So wrapping won't be a problem in that we will know which sids are available,
but it will be entirely possible to exhaust all sids or all memory with them.
So we have a builtin DOS, for users that can manipulate policy

Oh happy day

>> +struct aa_label *aa_label_alloc(int size, gfp_t gfp)
>> +{
>> +    struct aa_label *label;
>> +
>> +    AA_WARN(size < 1);
>> +
>> +    label = kzalloc(sizeof(*label) + sizeof(struct aa_label *) * (size - 1),
>> +                    gfp);
> 
> Can this be turned into BUG_ON? We'd never want size <= 0 here...
> 
all the AA_WARNS will switch to something more aggressive. It isn't bug_on atm
because bug_on actually makes it harder to debug, as half of these warnings
are in places with spin_irq locks held and if they bug, the system is dead.

>> @@ -915,7 +917,7 @@ static int replacement_allowed(struct aa_profile 
>> *profile, int noreplace,
>>                             const char **info)
>>  {
>>      if (profile) {
>> -            if (profile->flags & PFLAG_IMMUTABLE) {
>> +            if (profile->label.flags & FLAG_IMMUTABLE) {
>>                      *info = "cannot replace immutible profile";
>>                      return -EPERM;
>>              } else if (noreplace) {
> 
> "immutible"
> 
> 
> Thanks
> 
> 
> 


-- 
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/apparmor

Reply via email to