On Wed, 2026-07-22 at 10:40 -0700, Ryan Lee wrote: > On Wed, Jul 22, 2026 at 8:51 AM Georgia Garcia > <[email protected]> wrote: > > > > On Tue, 2026-07-07 at 11:13 -0700, Ryan Lee via AppArmor wrote: > > > Signed-off-by: Ryan Lee <[email protected]> > > > --- > > > security/apparmor/policy.c | 11 ++++++----- > > > 1 file changed, 6 insertions(+), 5 deletions(-) > > > > > > diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c > > > index 9475f6455542..be939a8332f6 100644 > > > --- a/security/apparmor/policy.c > > > +++ b/security/apparmor/policy.c > > > @@ -1138,21 +1138,22 @@ static void share_name(struct aa_profile *old, > > > struct aa_profile *new) > > > static struct aa_profile *update_to_newest_parent(struct aa_profile *new) > > > { > > > struct aa_profile *parent, *newest; > > > + bool needput; > > > > > > parent = rcu_dereference_protected(new->parent, > > > mutex_is_locked(&new->ns->lock)); > > > - newest = aa_get_newest_profile(parent); > > > + newest = aa_get_newest_profile_condref(parent, &needput); > > > > > > /* parent replaced in this atomic set? */ > > > if (newest != parent) { > > > rcu_assign_pointer(new->parent, newest); > > > } > > > - /* Unconditionally put the parent: > > > + /* put the parent instead of newest, if necessary: > > > * If parent != newest, we aren't using the parent anymore > > > - * If parent == newest, we got an extra ref to it, so put it and > > > - * retain the other ref we have to newest > > > > I think the code change looks good but this comment is more confusing > > than it needs to be. > > Could you explain how you'd make this comment less confusing? I > personally don't see how it would be confusing. >
It's just that if parent == newest, it's unlikely that the condref function will get a ref, so we just delegate the decision to put or not to the needput variable. There's nothing wrong with the comment, it's just that I had to read it a few times to grasp it. (Maybe I'm a little slow today :) ) > > > > > + * If parent == newest, we may have gotten an extra ref to it, > > > + * so put it and retain the other ref we have to newest > > > */ > > > - aa_put_profile(parent); > > > + aa_put_profile_condref(parent, needput); > > > > > > return newest; > > > } > >
