On 7/7/26 11:13, Ryan Lee wrote:
This is a preparatory cleanup for switching to a condref version of
refreshing the parent label.

Signed-off-by: Ryan Lee <[email protected]>

NAK,

while technically correct, it is being tricky, and causing
the code to be harder to understand. Keeping the put_profile(parent)
next to the new assignment ref transfer is much easier to understand.

and once we move to the cond version, you no longer have the extra
ref count so the aa_put_profile(newest) isn't going to cause
overhead.

Even then its better to treat the get_newest_condref as a critical
section block, separate from the logic of updating the reference
on the object.


---
  security/apparmor/policy.c | 10 +++++++---
  1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c
index f6f1b72d7c3d..9475f6455542 100644
--- a/security/apparmor/policy.c
+++ b/security/apparmor/policy.c
@@ -1145,10 +1145,14 @@ static struct aa_profile 
*update_to_newest_parent(struct aa_profile *new)
/* parent replaced in this atomic set? */
        if (newest != parent) {
-               aa_put_profile(parent);
                rcu_assign_pointer(new->parent, newest);
-       } else
-               aa_put_profile(newest);
+       }
+       /* Unconditionally put the parent:
+        * 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
+        */
+       aa_put_profile(parent);
return newest;
  }


Reply via email to