From d9087c49d4388e3f35f09a5cf7ed6e09c9106604 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Fri, 27 Jan 2017 03:53:53 -0800 Subject: [PATCH] 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 --- security/apparmor/context.c | 83 ++++++++++--------------------------- security/apparmor/domain.c | 14 +++---- security/apparmor/include/context.h | 24 +++-------- security/apparmor/lsm.c | 55 +++++++----------------- 4 files changed, 47 insertions(+), 129 deletions(-) diff --git a/security/apparmor/context.c b/security/apparmor/context.c index 432672b18945..70e4a094add8 100644 --- 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 b90759a765b5..5285938680e0 100644 --- 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 c3b51d88275b..8d36c14bc76d 100644 --- 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 a1d63d93b862..628c6a07df64 100644 --- 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) -- 2.11.0