[apparmor] [PATCH 07/15] apparmor: remove previous field from aa_task_cxt
Seth Arnold
seth.arnold at gmail.com
Thu Jul 12 21:26:31 UTC 2012
This block loses the aa_put_profile() before overwriting the pointer, is this intentional?
-aa_put_profile(cxt->profile);
-cxt->profile = aa_newest_version(cxt->previous);
+p = cxt->profile;
+cxt->profile = aa_get_profile(aa_newest_version(p->parent));
-----Original Message-----
From: John Johansen <john.johansen at canonical.com>
Sender: apparmor-bounces at lists.ubuntu.com
Date: Thu, 12 Jul 2012 12:05:03
To: <apparmor at lists.ubuntu.com>
Subject: [apparmor] [PATCH 07/15] apparmor: remove previous field from
aa_task_cxt
The previous field is used to store the parent profile during change_hat
but this is redundant information as every hat profile now stores a pointer
to its parent. Use profile->parent to replace previous.
As part of this factor out clearing on the task_cxt transition context
into its own fn.
Signed-off-by: John Johansen <john.johansen at canonical.com>
---
security/apparmor/context.c | 36 ++++++++++++-----------------------
security/apparmor/domain.c | 15 ++++-----------
security/apparmor/include/context.h | 15 ++++++++++++---
security/apparmor/lsm.c | 4 ++--
4 files changed, 30 insertions(+), 40 deletions(-)
diff --git a/security/apparmor/context.c b/security/apparmor/context.c
index 611e6ce..f084fb4 100644
--- a/security/apparmor/context.c
+++ b/security/apparmor/context.c
@@ -48,7 +48,6 @@ void aa_free_task_context(struct aa_task_cxt *cxt)
{
if (cxt) {
aa_put_profile(cxt->profile);
- aa_put_profile(cxt->previous);
aa_put_profile(cxt->onexec);
kzfree(cxt);
@@ -64,7 +63,6 @@ void aa_dup_task_context(struct aa_task_cxt *new, const struct aa_task_cxt *old)
{
*new = *old;
aa_get_profile(new->profile);
- aa_get_profile(new->previous);
aa_get_profile(new->onexec);
}
@@ -105,16 +103,12 @@ int aa_replace_current_profile(struct aa_profile *profile)
return -ENOMEM;
cxt = new->security;
- if (unconfined(profile) || (cxt->profile->ns != profile->ns)) {
+ if (unconfined(profile) || (cxt->profile->ns != profile->ns))
/* if switching to unconfined or a different profile namespace
* clear out context state
*/
- aa_put_profile(cxt->previous);
- aa_put_profile(cxt->onexec);
- cxt->previous = NULL;
- cxt->onexec = NULL;
- cxt->token = 0;
- }
+ aa_clear_task_cxt_trans(cxt);
+
/* be careful switching cxt->profile, when racing replacement it
* is possible that cxt->profile->replacedby is the reference keeping
* @profile valid, so make sure to get its reference before dropping
@@ -168,14 +162,12 @@ int aa_set_current_hat(struct aa_profile *profile, u64 token)
BUG_ON(!profile);
cxt = new->security;
- if (!cxt->previous) {
- /* transfer refcount */
- cxt->previous = cxt->profile;
+ if (!PROFILE_IS_HAT(cxt->profile)) {
cxt->token = token;
} else if (cxt->token == token) {
aa_put_profile(cxt->profile);
} else {
- /* previous_profile && cxt->token != token */
+ /* in hat && cxt->token != token */
abort_creds(new);
return -EACCES;
}
@@ -200,6 +192,7 @@ int aa_set_current_hat(struct aa_profile *profile, u64 token)
int aa_restore_previous_profile(u64 token)
{
struct aa_task_cxt *cxt;
+ struct aa_profile *p;
struct cred *new = prepare_creds();
if (!new)
return -ENOMEM;
@@ -210,23 +203,18 @@ int aa_restore_previous_profile(u64 token)
return -EACCES;
}
/* ignore restores when there is no saved profile */
- if (!cxt->previous) {
+ if (!PROFILE_IS_HAT(cxt->profile)) {
abort_creds(new);
return 0;
}
- aa_put_profile(cxt->profile);
- cxt->profile = aa_newest_version(cxt->previous);
+ p = cxt->profile;
+ cxt->profile = aa_get_profile(aa_newest_version(p->parent));
BUG_ON(!cxt->profile);
- if (unlikely(cxt->profile != cxt->previous)) {
- aa_get_profile(cxt->profile);
- aa_put_profile(cxt->previous);
- }
+ aa_put_profile(p);
+
/* clear exec && prev information when restoring to previous context */
- cxt->previous = NULL;
- cxt->token = 0;
- aa_put_profile(cxt->onexec);
- cxt->onexec = NULL;
+ aa_clear_task_cxt_trans(cxt);
commit_creds(new);
return 0;
diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index 35e6605..4028f9d 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -510,11 +510,7 @@ x_clear:
cxt->profile = new_profile;
/* clear out all temporary/transitional state from the context */
- aa_put_profile(cxt->previous);
- aa_put_profile(cxt->onexec);
- cxt->previous = NULL;
- cxt->onexec = NULL;
- cxt->token = 0;
+ aa_clear_task_cxt_trans(cxt);
audit:
error = aa_audit_file(profile, &perms, GFP_KERNEL, OP_EXEC, MAY_EXEC,
@@ -612,8 +608,7 @@ static char *new_compound_name(const char *n1, const char *n2)
int aa_change_hat(const char *hats[], int count, u64 token, bool permtest)
{
const struct cred *cred;
- struct aa_task_cxt *cxt;
- struct aa_profile *profile, *previous_profile, *hat = NULL;
+ struct aa_profile *profile, *hat = NULL;
char *name = NULL;
int i;
struct file_perms perms = {};
@@ -630,9 +625,7 @@ int aa_change_hat(const char *hats[], int count, u64 token, bool permtest)
/* released below */
cred = get_current_cred();
- cxt = cred->security;
profile = aa_cred_profile(cred);
- previous_profile = cxt->previous;
if (unconfined(profile)) {
info = "unconfined";
@@ -702,11 +695,11 @@ int aa_change_hat(const char *hats[], int count, u64 token, bool permtest)
/* reset error for learning of new hats */
error = -ENOENT;
}
- } else if (previous_profile) {
+ } else if (PROFILE_IS_HAT(profile)) {
/* Return to saved profile. Kill task if restore fails
* to avoid brute force attacks
*/
- target = previous_profile->base.hname;
+ target = profile->parent->base.hname;
error = aa_restore_previous_profile(token);
perms.kill = AA_MAY_CHANGEHAT;
} else
diff --git a/security/apparmor/include/context.h b/security/apparmor/include/context.h
index f5fe462f..315b94c 100644
--- a/security/apparmor/include/context.h
+++ b/security/apparmor/include/context.h
@@ -57,8 +57,7 @@ static inline void aa_free_file_context(struct aa_file_cxt *cxt)
* struct aa_task_cxt - primary label for confined tasks
* @profile: the current profile (NOT NULL)
* @exec: profile to transition to on next exec (MAYBE NULL)
- * @previous: profile the task may return to (MAYBE NULL)
- * @token: magic value the task must know for returning to @previous_profile
+ * @token: magic value the task must know for returning to @parent
*
* Contains the task's current profile (which could change due to
* change_hat). Plus the hat_magic needed during change_hat.
@@ -68,7 +67,6 @@ static inline void aa_free_file_context(struct aa_file_cxt *cxt)
struct aa_task_cxt {
struct aa_profile *profile;
struct aa_profile *onexec;
- struct aa_profile *previous;
u64 token;
};
@@ -163,4 +161,15 @@ static inline struct aa_profile *aa_current_profile(void)
return profile;
}
+/**
+ * aa_clear_task_cxt_trans - clear transition tracking info from the cxt
+ * @cxt: task context to clear (NOT NULL)
+ */
+static inline void aa_clear_task_cxt_trans(struct aa_task_cxt *cxt)
+{
+ aa_put_profile(cxt->onexec);
+ cxt->onexec = NULL;
+ cxt->token = 0;
+}
+
#endif /* __AA_CONTEXT_H */
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 5a21d2a..52021c9 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -512,8 +512,8 @@ static int apparmor_getprocattr(struct task_struct *task, char *name,
if (strcmp(name, "current") == 0)
error = aa_getprocattr(aa_newest_version(cxt->profile),
value);
- else if (strcmp(name, "prev") == 0 && cxt->previous)
- error = aa_getprocattr(aa_newest_version(cxt->previous),
+ else if (strcmp(name, "prev") == 0 && PROFILE_IS_HAT(cxt->profile))
+ error = aa_getprocattr(aa_newest_version(cxt->profile->parent),
value);
else if (strcmp(name, "exec") == 0 && cxt->onexec)
error = aa_getprocattr(aa_newest_version(cxt->onexec),
--
1.7.9.5
--
AppArmor mailing list
AppArmor at lists.ubuntu.com
Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
More information about the AppArmor
mailing list