apparmor: check label build before no_new_privs test

aa_change_profile() builds a replacement label with
fn_label_build_in_scope() before the no_new_privs subset check. The build
helper can fail and return NULL or an ERR_PTR, but the result was passed
to aa_label_is_unconfined_subset() before the existing IS_ERR_OR_NULL()
check.

Reuse the existing target-label build failure handling immediately after
the build. This preserves the current audit handling while preventing the
subset helper from dereferencing an invalid label.

Fixes: e00b02bb6ac2a ("apparmor: move change_profile mediation to using labels")
Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>

diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index f02bf770f6385437946b24e97f85e8ab7d1182b0..6748ac74b060bd58924f54e7aef159b040b0364f 100644 (file)
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -1527,6 +1527,8 @@ check:
                new = fn_label_build_in_scope(label, profile, GFP_KERNEL,
                                           aa_get_label(target),
                                           aa_get_label(&profile->label));
+               if (IS_ERR_OR_NULL(new))
+                       goto build_fail;
                /*
                 * no new privs prevents domain transitions that would
                 * reduce restrictions.
@@ -1545,16 +1547,8 @@ check:
                /* only transition profiles in the current ns */
                if (stack)
                        new = aa_label_merge(label, target, GFP_KERNEL);
-               if (IS_ERR_OR_NULL(new)) {
-                       info = "failed to build target label";
-                       if (!new)
-                               error = -ENOMEM;
-                       else
-                               error = PTR_ERR(new);
-                       new = NULL;
-                       perms.allow = 0;
-                       goto audit;
-               }
+               if (IS_ERR_OR_NULL(new))
+                       goto build_fail;
                error = aa_replace_current_label(new);
        } else {
                if (new) {
@@ -1566,6 +1560,17 @@ check:
                aa_set_current_onexec(target, stack);
        }
 
+       goto audit;
+
+build_fail:
+       info = "failed to build target label";
+       if (!new)
+               error = -ENOMEM;
+       else
+               error = PTR_ERR(new);
+       new = NULL;
+       perms.allow = 0;
+
 audit:
        error = fn_for_each_in_scope(label, profile,
                        aa_audit_file(subj_cred,