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