apparmor: drop cred_ctx and reference the label directly

With the task domain change information now stored in the task->security
context, the cred->security context only stores the label. We can get
rid of the cred_ctx and directly reference the label, removing a layer
of indirection, and unneeded extra allocations.

Signed-off-by: John Johansen <john.johansen@canonical.com>

diff --git a/security/apparmor/context.c b/security/apparmor/context.c
index 432672b18945b600c205146f385a0ce4ca3ea588..70e4a094add85378c3916db3acda409e867d5ba0 100644 (file)
--- a/security/apparmor/context.c
+++ b/security/apparmor/context.c
@@ -13,11 +13,9 @@
  * License.
  *
  *
- * AppArmor sets confinement on every task, via the the aa_cred_ctx and
- * the aa_cred_ctx.label, both of which are required and are not allowed
- * to be NULL.  The aa_cred_ctx is not reference counted and is unique
- * to each cred (which is reference count).  The label pointed to by
- * the cred_ctx is reference counted.
+ * AppArmor sets confinement on every task, via the cred_label() which
+ * is required and are not allowed to be NULL.  The cred_label is
+ * reference counted.
  *
  * TODO
  * If a task uses change_hat it currently does not return to the old
@@ -29,40 +27,6 @@
 #include "include/context.h"
 #include "include/policy.h"
 
-/**
- * aa_alloc_cred_ctx - allocate a new cred_ctx
- * @flags: gfp flags for allocation
- *
- * Returns: allocated buffer or NULL on failure
- */
-struct aa_cred_ctx *aa_alloc_cred_ctx(gfp_t flags)
-{
-       return kzalloc(sizeof(struct aa_cred_ctx), flags);
-}
-
-/**
- * aa_free_cred_ctx - free a cred_ctx
- * @ctx: cred_ctx to free (MAYBE NULL)
- */
-void aa_free_cred_ctx(struct aa_cred_ctx *ctx)
-{
-       if (ctx) {
-               aa_put_label(ctx->label);
-
-               kzfree(ctx);
-       }
-}
-
-/**
- * aa_dup_cred_ctx - duplicate a task context, incrementing reference counts
- * @new: a blank task context      (NOT NULL)
- * @old: the task context to copy  (NOT NULL)
- */
-void aa_dup_cred_ctx(struct aa_cred_ctx *new, const struct aa_cred_ctx *old)
-{
-       *new = *old;
-       aa_get_label(new->label);
-}
 
 /**
  * aa_get_task_label - Get another task's label
@@ -126,11 +90,12 @@ void aa_dup_task_ctx(struct aa_task_ctx *new, const struct aa_task_ctx *old)
  */
 int aa_replace_current_label(struct aa_label *label)
 {
-       struct aa_cred_ctx *ctx = current_cred_ctx();
+       struct aa_label *old = aa_current_raw_label();
        struct cred *new;
+
        AA_BUG(!label);
 
-       if (ctx->label == label)
+       if (old == label)
                return 0;
 
        if (current_cred() != current_real_cred())
@@ -140,22 +105,22 @@ int aa_replace_current_label(struct aa_label *label)
        if (!new)
                return -ENOMEM;
 
-       ctx = cred_ctx(new);
-       if (unconfined(label) || (labels_ns(ctx->label) != labels_ns(label)))
-               /* if switching to unconfined or a different label namespace
+       if (unconfined(label) || (labels_ns(old) != labels_ns(label)))
+               /*
+                * if switching to unconfined or a different label namespace
                 * clear out context state
                 */
                aa_clear_task_ctx_trans(current_task_ctx());
 
        /*
-        * be careful switching ctx->profile, when racing replacement it
-        * is possible that ctx->profile->proxy->profile is the reference
-        * keeping @profile valid, so make sure to get its reference before
-        * dropping the reference on ctx->profile
+        * be careful switching cred label, when racing replacement it
+        * is possible that the cred labels's->proxy->label is the reference
+        * keeping @label valid, so make sure to get its reference before
+        * dropping the reference on the cred's label
         */
        aa_get_label(label);
-       aa_put_label(ctx->label);
-       ctx->label = label;
+       aa_put_label(cred_label(new));
+       cred_label(new) = label;
 
        commit_creds(new);
        return 0;
@@ -193,26 +158,26 @@ int aa_set_current_hat(struct aa_label *label, u64 token)
 {
        struct aa_task_ctx *tctx = current_task_ctx();
        struct aa_cred_ctx *ctx;
-       struct cred *new = prepare_creds();
+       struct cred *new;
 
+       new = prepare_creds();
        if (!new)
                return -ENOMEM;
        AA_BUG(!label);
 
-       ctx = cred_ctx(new);
        if (!tctx->previous) {
                /* transfer refcount */
-               tctx->previous = ctx->label;
+               tctx->previous = cred_label(new);
                tctx->token = token;
        } else if (tctx->token == token) {
-               aa_put_label(ctx->label);
+               aa_put_label(cred_label(new));
        } else {
                /* previous_profile && ctx->token != token */
                abort_creds(new);
                return -EACCES;
        }
 
-       ctx->label = aa_get_newest_label(label);
+       cred_label(new) = aa_get_newest_label(label);
        /* clear exec on switching context */
        aa_put_label(tctx->onexec);
        tctx->onexec = NULL;
@@ -233,7 +198,6 @@ int aa_set_current_hat(struct aa_label *label, u64 token)
 int aa_restore_previous_label(u64 token)
 {
        struct aa_task_ctx *tctx = current_task_ctx();
-       struct aa_cred_ctx *ctx;
        struct cred *new;
 
        if (tctx->token != token)
@@ -245,11 +209,10 @@ int aa_restore_previous_label(u64 token)
        new = prepare_creds();
        if (!new)
                return -ENOMEM;
-       ctx = cred_ctx(new);
 
-       aa_put_label(ctx->label);
-       ctx->label = aa_get_newest_label(tctx->previous);
-       AA_BUG(!ctx->label);
+       aa_put_label(cred_label(new));
+       cred_label(new) = aa_get_newest_label(tctx->previous);
+       AA_BUG(!cred_label(new));
        /* clear exec && prev information when restoring to previous context */
        aa_clear_task_ctx_trans(tctx);
 

diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index b90759a765b5c9dea4e3a857988109eba7b257b3..5285938680e09dce2810912016f23a46d9861d5d 100644 (file)
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -779,7 +779,6 @@ static struct aa_label *handle_onexec(struct aa_label *label,
  */
 int apparmor_bprm_set_creds(struct linux_binprm *bprm)
 {
-       struct aa_cred_ctx *ctx;
        struct aa_task_ctx *tctx;
        struct aa_label *label, *new = NULL;
        struct aa_profile *profile;
@@ -795,12 +794,11 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
        if (bprm->called_set_creds)
                return 0;
 
-       ctx = cred_ctx(bprm->cred);
        tctx = current_task_ctx();
-       AA_BUG(!ctx);
+       AA_BUG(!cred_label(bprm->cred));
        AA_BUG(!tctx);
 
-       label = aa_get_newest_label(ctx->label);
+       label = aa_get_newest_label(cred_label(bprm->cred));
 
        /* buffer freed below, name is pointer into buffer */
        get_buffers(buffer);
@@ -856,9 +854,9 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
                }
                bprm->per_clear |= PER_CLEAR_ON_SETID;
        }
-       aa_put_label(ctx->label);
-       /* transfer reference, released when ctx is freed */
-       ctx->label = new;
+       aa_put_label(cred_label(bprm->cred));
+       /* transfer reference, released when cred is freed */
+       cred_label(bprm->cred) = new;
 
 done:
        aa_put_label(label);
@@ -1049,7 +1047,6 @@ build:
 int aa_change_hat(const char *hats[], int count, u64 token, int flags)
 {
        const struct cred *cred;
-       struct aa_cred_ctx *ctx;
        struct aa_task_ctx *tctx;
        struct aa_label *label, *previous, *new = NULL, *target = NULL;
        struct aa_profile *profile;
@@ -1070,7 +1067,6 @@ int aa_change_hat(const char *hats[], int count, u64 token, int flags)
 
        /* released below */
        cred = get_current_cred();
-       ctx = cred_ctx(cred);
        tctx = current_task_ctx();
        label = aa_get_newest_cred_label(cred);
        previous = aa_get_newest_label(tctx->previous);

diff --git a/security/apparmor/include/context.h b/security/apparmor/include/context.h
index c3b51d88275bf55ccb933dcf0df0226376efab4a..8d36c14bc76d621f3d37f135cd87eab941f96e6c 100644 (file)
--- a/security/apparmor/include/context.h
+++ b/security/apparmor/include/context.h
@@ -22,21 +22,11 @@
 #include "label.h"
 #include "policy_ns.h"
 
-#define cred_ctx(X) ((X)->security)
-#define current_cred_ctx() cred_ctx(current_cred())
-
 #define task_ctx(X) ((X)->security)
 #define current_task_ctx() (task_ctx(current))
+#define cred_label(X) ((X)->security)
 
-/**
- * struct aa_cred_ctx - primary label for confined tasks
- * @label: the current label   (NOT NULL)
- */
-struct aa_cred_ctx {
-       struct aa_label *label;
-};
-
-/**
+/*
  * struct aa_task_ctx - information for current task label change
  * @onexec: profile to transition to on next exec  (MAY BE NULL)
  * @previous: profile the task may return to     (MAY BE NULL)
@@ -48,10 +38,6 @@ struct aa_task_ctx {
        u64 token;
 };
 
-struct aa_cred_ctx *aa_alloc_cred_ctx(gfp_t flags);
-void aa_free_cred_ctx(struct aa_cred_ctx *ctx);
-void aa_dup_cred_ctx(struct aa_cred_ctx *new, const struct aa_cred_ctx *old);
-
 struct aa_task_ctx *aa_alloc_task_ctx(gfp_t flags);
 void aa_free_task_ctx(struct aa_task_ctx *ctx);
 void aa_dup_task_ctx(struct aa_task_ctx *new, const struct aa_task_ctx *old);
@@ -73,10 +59,10 @@ struct aa_label *aa_get_task_label(struct task_struct *task);
  */
 static inline struct aa_label *aa_cred_raw_label(const struct cred *cred)
 {
-       struct aa_cred_ctx *ctx = cred_ctx(cred);
+       struct aa_label *label = cred_label(cred);
 
-       AA_BUG(!ctx || !ctx->label);
-       return ctx->label;
+       AA_BUG(!label);
+       return label;
 }
 
 /**

diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index a1d63d93b862d52fbf5037ec8eba0b160bf06688..628c6a07df647892d7db80064b7fe37cccbfa857 100644 (file)
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -51,12 +51,12 @@ DEFINE_PER_CPU(struct aa_buffers, aa_buffers);
  */
 
 /*
- * free the associated aa_cred_ctx and put its labels
+ * put the associated labels
  */
 static void apparmor_cred_free(struct cred *cred)
 {
-       aa_free_cred_ctx(cred_ctx(cred));
-       cred_ctx(cred) = NULL;
+       aa_put_label(cred_label(cred));
+       cred_label(cred) = NULL;
 }
 
 /*
@@ -64,30 +64,17 @@ static void apparmor_cred_free(struct cred *cred)
  */
 static int apparmor_cred_alloc_blank(struct cred *cred, gfp_t gfp)
 {
-       /* freed by apparmor_cred_free */
-       struct aa_cred_ctx *ctx = aa_alloc_cred_ctx(gfp);
-
-       if (!ctx)
-               return -ENOMEM;
-
-       cred_ctx(cred) = ctx;
+       cred_label(cred) = NULL;
        return 0;
 }
 
 /*
- * prepare new aa_cred_ctx for modification by prepare_cred block
+ * prepare new cred label for modification by prepare_cred block
  */
 static int apparmor_cred_prepare(struct cred *new, const struct cred *old,
                                 gfp_t gfp)
 {
-       /* freed by apparmor_cred_free */
-       struct aa_cred_ctx *ctx = aa_alloc_cred_ctx(gfp);
-
-       if (!ctx)
-               return -ENOMEM;
-
-       aa_dup_cred_ctx(ctx, cred_ctx(old));
-       cred_ctx(new) = ctx;
+       cred_label(new) = aa_get_newest_label(cred_label(old));
        return 0;
 }
 
@@ -96,10 +83,7 @@ static int apparmor_cred_prepare(struct cred *new, const struct cred *old,
  */
 static void apparmor_cred_transfer(struct cred *new, const struct cred *old)
 {
-       const struct aa_cred_ctx *old_ctx = cred_ctx(old);
-       struct aa_cred_ctx *new_ctx = cred_ctx(new);
-
-       aa_dup_cred_ctx(new_ctx, old_ctx);
+       cred_label(new) = aa_get_newest_label(cred_label(old));
 }
 
 static void apparmor_task_free(struct task_struct *task)
@@ -599,11 +583,10 @@ static int apparmor_getprocattr(struct task_struct *task, char *name,
        /* released below */
        const struct cred *cred = get_task_cred(task);
        struct aa_task_ctx *tctx = current_task_ctx();
-       struct aa_cred_ctx *ctx = cred_ctx(cred);
        struct aa_label *label = NULL;
 
        if (strcmp(name, "current") == 0)
-               label = aa_get_newest_label(ctx->label);
+               label = aa_get_newest_label(cred_label(cred));
        else if (strcmp(name, "prev") == 0  && tctx->previous)
                label = aa_get_newest_label(tctx->previous);
        else if (strcmp(name, "exec") == 0 && tctx->onexec)
@@ -700,11 +683,11 @@ fail:
 static void apparmor_bprm_committing_creds(struct linux_binprm *bprm)
 {
        struct aa_label *label = aa_current_raw_label();
-       struct aa_cred_ctx *new_ctx = cred_ctx(bprm->cred);
+       struct aa_label *new_label = cred_label(bprm->cred);
 
        /* bail out if unconfined or not changing profile */
-       if ((new_ctx->label->proxy == label->proxy) ||
-           (unconfined(new_ctx->label)))
+       if ((new_label->proxy == label->proxy) ||
+           (unconfined(new_label)))
                return;
 
        aa_inherit_files(bprm->cred, current->files);
@@ -712,7 +695,7 @@ static void apparmor_bprm_committing_creds(struct linux_binprm *bprm)
        current->pdeath_signal = 0;
 
        /* reset soft limits and set hard limits for the new label */
-       __aa_transition_rlimits(label, new_ctx->label);
+       __aa_transition_rlimits(label, new_label);
 }
 
 /**
@@ -1050,26 +1033,16 @@ static int param_set_mode(const char *val, const struct kernel_param *kp)
 static int __init set_init_ctx(void)
 {
        struct cred *cred = (struct cred *)current->real_cred;
-       struct aa_cred_ctx *ctx;
        struct aa_task_ctx *tctx;
 
-       ctx = aa_alloc_cred_ctx(GFP_KERNEL);
-       if (!ctx)
-               goto fail_cred;
        tctx = aa_alloc_task_ctx(GFP_KERNEL);
        if (!tctx)
-               goto fail_task;
+               return -ENOMEM;
 
-       ctx->label = aa_get_label(ns_unconfined(root_ns));
-       cred_ctx(cred) = ctx;
+       cred_label(cred) = aa_get_label(ns_unconfined(root_ns));
        task_ctx(current) = tctx;
 
        return 0;
-
-fail_task:
-       aa_free_cred_ctx(ctx);
-fail_cred:
-       return -ENOMEM;
 }
 
 static void destroy_buffers(void)