From 17322cc3f9ba578f20b5c09fb1630bd234040008 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Mon, 18 Feb 2013 15:59:34 -0800 Subject: apparmor: fix auditing of domain transition failures due to incomplete policy When policy specifies a transition to a profile that is not currently loaded, it result in exec being denied. However the failure is not being audited correctly because the audit code is treating this as an allowed permission and thus not reporting it. Signed-off-by: John Johansen Acked-By: Steve Beattie --- security/apparmor/domain.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'security') diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index 859abdaac1ea..7bc85c7f4573 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -443,6 +443,8 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm) } else { error = -ENOENT; info = "profile not found"; + /* remove MAY_EXEC to audit as failure */ + perms.allow &= ~MAY_EXEC; } } } else if (COMPLAIN_MODE(profile)) { -- cgit v1.2.3-59-g8ed1b From 04266236b1c3030bb7f75472ac85a8b78fcfb284 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Mon, 18 Feb 2013 16:00:34 -0800 Subject: apparmor: Remove -W1 warnings Signed-off-by: John Johansen Acked-By: Steve Beattie --- security/apparmor/domain.c | 2 -- security/apparmor/lsm.c | 4 ---- 2 files changed, 6 deletions(-) (limited to 'security') diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index 7bc85c7f4573..7a78e814f0d4 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -752,7 +752,6 @@ int aa_change_profile(const char *ns_name, const char *hname, bool onexec, bool permtest) { const struct cred *cred; - struct aa_task_cxt *cxt; struct aa_profile *profile, *target = NULL; struct aa_namespace *ns = NULL; struct file_perms perms = {}; @@ -772,7 +771,6 @@ int aa_change_profile(const char *ns_name, const char *hname, bool onexec, } cred = get_current_cred(); - cxt = cred->security; profile = aa_cred_profile(cred); /* diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index b21830eced41..0f61dadca9e6 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -469,7 +469,6 @@ static int apparmor_file_lock(struct file *file, unsigned int cmd) static int common_mmap(int op, struct file *file, unsigned long prot, unsigned long flags) { - struct dentry *dentry; int mask = 0; if (!file || !file->f_security) @@ -486,7 +485,6 @@ static int common_mmap(int op, struct file *file, unsigned long prot, if (prot & PROT_EXEC) mask |= AA_EXEC_MMAP; - dentry = file->f_path.dentry; return common_file_perm(op, file, mask); } @@ -507,11 +505,9 @@ static int apparmor_getprocattr(struct task_struct *task, char *name, char **value) { int error = -ENOENT; - struct aa_profile *profile; /* released below */ const struct cred *cred = get_task_cred(task); struct aa_task_cxt *cxt = cred->security; - profile = aa_cred_profile(cred); if (strcmp(name, "current") == 0) error = aa_getprocattr(aa_newest_version(cxt->profile), -- cgit v1.2.3-59-g8ed1b From 50c5ecd5d8ffb0e549676b8fd9781e3b2fd751a0 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Mon, 18 Feb 2013 16:01:34 -0800 Subject: apparmor: refactor profile mode macros Signed-off-by: John Johansen Acked-by: Steve Beattie --- security/apparmor/include/policy.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'security') diff --git a/security/apparmor/include/policy.h b/security/apparmor/include/policy.h index bda4569fdd83..95979c431e26 100644 --- a/security/apparmor/include/policy.h +++ b/security/apparmor/include/policy.h @@ -32,13 +32,13 @@ extern const char *const profile_mode_names[]; #define APPARMOR_NAMES_MAX_INDEX 3 -#define COMPLAIN_MODE(_profile) \ - ((aa_g_profile_mode == APPARMOR_COMPLAIN) || \ - ((_profile)->mode == APPARMOR_COMPLAIN)) +#define PROFILE_MODE(_profile, _mode) \ + ((aa_g_profile_mode == (_mode)) || \ + ((_profile)->mode == (_mode))) -#define KILL_MODE(_profile) \ - ((aa_g_profile_mode == APPARMOR_KILL) || \ - ((_profile)->mode == APPARMOR_KILL)) +#define COMPLAIN_MODE(_profile) PROFILE_MODE((_profile), APPARMOR_COMPLAIN) + +#define KILL_MODE(_profile) PROFILE_MODE((_profile), APPARMOR_KILL) #define PROFILE_IS_HAT(_profile) ((_profile)->flags & PFLAG_HAT) -- cgit v1.2.3-59-g8ed1b From e573cc30bb36df23fb49a29d96e6c6333d17f59c Mon Sep 17 00:00:00 2001 From: John Johansen Date: Mon, 18 Feb 2013 16:02:34 -0800 Subject: apparmor: fix error code to failure message mapping for name lookup -ESTALE used to be incorrectly used to indicate a disconnected path, when name lookup failed. This was fixed in commit e1b0e444 to correctly return -EACCESS, but the error to failure message mapping was not correctly updated to reflect this change. Signed-off-by: John Johansen Acked-by: Steve Beattie --- security/apparmor/path.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security') diff --git a/security/apparmor/path.c b/security/apparmor/path.c index e91ffee80162..35b394a75d76 100644 --- a/security/apparmor/path.c +++ b/security/apparmor/path.c @@ -174,7 +174,7 @@ static int get_name_to_buffer(struct path *path, int flags, char *buffer, if (info && error) { if (error == -ENOENT) *info = "Failed name lookup - deleted entry"; - else if (error == -ESTALE) + else if (error == -EACCES) *info = "Failed name lookup - disconnected path"; else if (error == -ENAMETOOLONG) *info = "Failed name lookup - name too long"; -- cgit v1.2.3-59-g8ed1b From 3cfcc19e0b5390c04cb5bfa4e8fde39395410e61 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Mon, 18 Feb 2013 16:03:34 -0800 Subject: apparmor: add utility function to get an arbitrary tasks profile. Signed-off-by: John Johansen Acked-by: Steve Beattie --- security/apparmor/context.c | 17 +++++++++++++++ security/apparmor/domain.c | 10 +++------ security/apparmor/include/context.h | 41 ++++++++++++++++++++++--------------- security/apparmor/ipc.c | 13 ++++-------- 4 files changed, 49 insertions(+), 32 deletions(-) (limited to 'security') diff --git a/security/apparmor/context.c b/security/apparmor/context.c index 8a9b5027c813..611e6ce70b03 100644 --- a/security/apparmor/context.c +++ b/security/apparmor/context.c @@ -68,6 +68,23 @@ void aa_dup_task_context(struct aa_task_cxt *new, const struct aa_task_cxt *old) aa_get_profile(new->onexec); } +/** + * aa_get_task_profile - Get another task's profile + * @task: task to query (NOT NULL) + * + * Returns: counted reference to @task's profile + */ +struct aa_profile *aa_get_task_profile(struct task_struct *task) +{ + struct aa_profile *p; + + rcu_read_lock(); + p = aa_get_profile(__aa_task_profile(task)); + rcu_read_unlock(); + + return p; +} + /** * aa_replace_current_profile - replace the current tasks profiles * @profile: new profile (NOT NULL) diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index 7a78e814f0d4..fb47d5b71ea6 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -62,17 +62,14 @@ static int may_change_ptraced_domain(struct task_struct *task, struct aa_profile *to_profile) { struct task_struct *tracer; - const struct cred *cred = NULL; struct aa_profile *tracerp = NULL; int error = 0; rcu_read_lock(); tracer = ptrace_parent(task); - if (tracer) { + if (tracer) /* released below */ - cred = get_task_cred(tracer); - tracerp = aa_cred_profile(cred); - } + tracerp = aa_get_task_profile(tracer); /* not ptraced */ if (!tracer || unconfined(tracerp)) @@ -82,8 +79,7 @@ static int may_change_ptraced_domain(struct task_struct *task, out: rcu_read_unlock(); - if (cred) - put_cred(cred); + aa_put_profile(tracerp); return error; } diff --git a/security/apparmor/include/context.h b/security/apparmor/include/context.h index a9cbee4d9e48..1e9443a58877 100644 --- a/security/apparmor/include/context.h +++ b/security/apparmor/include/context.h @@ -80,23 +80,8 @@ int aa_replace_current_profile(struct aa_profile *profile); int aa_set_current_onexec(struct aa_profile *profile); int aa_set_current_hat(struct aa_profile *profile, u64 token); int aa_restore_previous_profile(u64 cookie); +struct aa_profile *aa_get_task_profile(struct task_struct *task); -/** - * __aa_task_is_confined - determine if @task has any confinement - * @task: task to check confinement of (NOT NULL) - * - * If @task != current needs to be called in RCU safe critical section - */ -static inline bool __aa_task_is_confined(struct task_struct *task) -{ - struct aa_task_cxt *cxt = __task_cred(task)->security; - - BUG_ON(!cxt || !cxt->profile); - if (unconfined(aa_newest_version(cxt->profile))) - return 0; - - return 1; -} /** * aa_cred_profile - obtain cred's profiles @@ -113,6 +98,30 @@ static inline struct aa_profile *aa_cred_profile(const struct cred *cred) return aa_newest_version(cxt->profile); } +/** + * __aa_task_profile - retrieve another task's profile + * @task: task to query (NOT NULL) + * + * Returns: @task's profile without incrementing its ref count + * + * If @task != current needs to be called in RCU safe critical section + */ +static inline struct aa_profile *__aa_task_profile(struct task_struct *task) +{ + return aa_cred_profile(__task_cred(task)); +} + +/** + * __aa_task_is_confined - determine if @task has any confinement + * @task: task to check confinement of (NOT NULL) + * + * If @task != current needs to be called in RCU safe critical section + */ +static inline bool __aa_task_is_confined(struct task_struct *task) +{ + return !unconfined(__aa_task_profile(task)); +} + /** * __aa_current_profile - find the current tasks confining profile * diff --git a/security/apparmor/ipc.c b/security/apparmor/ipc.c index cf1071b14232..c51d2266587e 100644 --- a/security/apparmor/ipc.c +++ b/security/apparmor/ipc.c @@ -95,23 +95,18 @@ int aa_ptrace(struct task_struct *tracer, struct task_struct *tracee, * - tracer profile has CAP_SYS_PTRACE */ - struct aa_profile *tracer_p; - /* cred released below */ - const struct cred *cred = get_task_cred(tracer); + struct aa_profile *tracer_p = aa_get_task_profile(tracer); int error = 0; - tracer_p = aa_cred_profile(cred); if (!unconfined(tracer_p)) { - /* lcred released below */ - const struct cred *lcred = get_task_cred(tracee); - struct aa_profile *tracee_p = aa_cred_profile(lcred); + struct aa_profile *tracee_p = aa_get_task_profile(tracee); error = aa_may_ptrace(tracer, tracer_p, tracee_p, mode); error = aa_audit_ptrace(tracer_p, tracee_p, error); - put_cred(lcred); + aa_put_profile(tracee_p); } - put_cred(cred); + aa_put_profile(tracer_p); return error; } -- cgit v1.2.3-59-g8ed1b From 0ca554b9fca425eb58325a36290deef698cef34b Mon Sep 17 00:00:00 2001 From: John Johansen Date: Mon, 18 Feb 2013 16:04:34 -0800 Subject: apparmor: add kvzalloc to handle zeroing for kvmalloc Signed-off-by: John Johansen Acked-by: Steve Beattie --- security/apparmor/include/apparmor.h | 12 +++++++++++- security/apparmor/lib.c | 14 +++++++++----- security/apparmor/match.c | 4 ++-- 3 files changed, 22 insertions(+), 8 deletions(-) (limited to 'security') diff --git a/security/apparmor/include/apparmor.h b/security/apparmor/include/apparmor.h index 40aedd9f73ea..1ba2ca56a6ef 100644 --- a/security/apparmor/include/apparmor.h +++ b/security/apparmor/include/apparmor.h @@ -15,6 +15,7 @@ #ifndef __APPARMOR_H #define __APPARMOR_H +#include #include #include "match.h" @@ -64,9 +65,18 @@ extern int apparmor_initialized __initdata; /* fn's in lib */ char *aa_split_fqname(char *args, char **ns_name); void aa_info_message(const char *str); -void *kvmalloc(size_t size); +void *__aa_kvmalloc(size_t size, gfp_t flags); void kvfree(void *buffer); +static inline void *kvmalloc(size_t size) +{ + return __aa_kvmalloc(size, 0); +} + +static inline void *kvzalloc(size_t size) +{ + return __aa_kvmalloc(size, __GFP_ZERO); +} /** * aa_strneq - compare null terminated @str to a non null terminated substring diff --git a/security/apparmor/lib.c b/security/apparmor/lib.c index 7430298116d6..d6e1f2148398 100644 --- a/security/apparmor/lib.c +++ b/security/apparmor/lib.c @@ -75,15 +75,16 @@ void aa_info_message(const char *str) } /** - * kvmalloc - do allocation preferring kmalloc but falling back to vmalloc - * @size: size of allocation + * __aa_kvmalloc - do allocation preferring kmalloc but falling back to vmalloc + * @size: how many bytes of memory are required + * @flags: the type of memory to allocate (see kmalloc). * * Return: allocated buffer or NULL if failed * * It is possible that policy being loaded from the user is larger than * what can be allocated by kmalloc, in those cases fall back to vmalloc. */ -void *kvmalloc(size_t size) +void *__aa_kvmalloc(size_t size, gfp_t flags) { void *buffer = NULL; @@ -92,14 +93,17 @@ void *kvmalloc(size_t size) /* do not attempt kmalloc if we need more than 16 pages at once */ if (size <= (16*PAGE_SIZE)) - buffer = kmalloc(size, GFP_NOIO | __GFP_NOWARN); + buffer = kmalloc(size, flags | GFP_NOIO | __GFP_NOWARN); if (!buffer) { /* see kvfree for why size must be at least work_struct size * when allocated via vmalloc */ if (size < sizeof(struct work_struct)) size = sizeof(struct work_struct); - buffer = vmalloc(size); + if (flags & __GFP_ZERO) + buffer = vzalloc(size); + else + buffer = vmalloc(size); } return buffer; } diff --git a/security/apparmor/match.c b/security/apparmor/match.c index 90971a8c3789..dfd25a9c9a69 100644 --- a/security/apparmor/match.c +++ b/security/apparmor/match.c @@ -30,7 +30,7 @@ * * Returns: pointer to table else NULL on failure * - * NOTE: must be freed by kvfree (not kmalloc) + * NOTE: must be freed by kvfree (not kfree) */ static struct table_header *unpack_table(char *blob, size_t bsize) { @@ -57,7 +57,7 @@ static struct table_header *unpack_table(char *blob, size_t bsize) if (bsize < tsize) goto out; - table = kvmalloc(tsize); + table = kvzalloc(tsize); if (table) { *table = th; if (th.td_flags == YYTD_DATA8) -- cgit v1.2.3-59-g8ed1b From 7a2871b566f34d980556072943295efd107eb53c Mon Sep 17 00:00:00 2001 From: John Johansen Date: Mon, 18 Feb 2013 16:05:34 -0800 Subject: apparmor: use common fn to clear task_context for domain transitions Signed-off-by: John Johansen Acked-by: Steve Beattie --- security/apparmor/context.c | 17 ++++++----------- security/apparmor/domain.c | 6 +----- security/apparmor/include/context.h | 13 +++++++++++++ 3 files changed, 20 insertions(+), 16 deletions(-) (limited to 'security') diff --git a/security/apparmor/context.c b/security/apparmor/context.c index 611e6ce70b03..3f911afa2bb9 100644 --- a/security/apparmor/context.c +++ b/security/apparmor/context.c @@ -105,16 +105,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 @@ -222,11 +218,10 @@ int aa_restore_previous_profile(u64 token) aa_get_profile(cxt->profile); aa_put_profile(cxt->previous); } - /* clear exec && prev information when restoring to previous context */ + /* ref has been transfered so avoid putting ref in clear_task_cxt */ cxt->previous = NULL; - cxt->token = 0; - aa_put_profile(cxt->onexec); - cxt->onexec = NULL; + /* clear exec && prev information when restoring to previous context */ + aa_clear_task_cxt_trans(cxt); commit_creds(new); return 0; diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index fb47d5b71ea6..07fcb09b990f 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -512,11 +512,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, diff --git a/security/apparmor/include/context.h b/security/apparmor/include/context.h index 1e9443a58877..4cecad313227 100644 --- a/security/apparmor/include/context.h +++ b/security/apparmor/include/context.h @@ -160,4 +160,17 @@ 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->previous); + aa_put_profile(cxt->onexec); + cxt->previous = NULL; + cxt->onexec = NULL; + cxt->token = 0; +} + #endif /* __AA_CONTEXT_H */ -- cgit v1.2.3-59-g8ed1b From 4b7c331fc2eceaa4da5ded41c0b2eca3fd924444 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Mon, 18 Feb 2013 16:06:34 -0800 Subject: apparmor: remove "permipc" command The "permipc" command is unused and unfinished, remove it. Signed-off-by: John Johansen Acked-by: Kees Cook --- security/apparmor/include/procattr.h | 1 - security/apparmor/lsm.c | 2 -- security/apparmor/procattr.c | 6 ------ 3 files changed, 9 deletions(-) (limited to 'security') diff --git a/security/apparmor/include/procattr.h b/security/apparmor/include/procattr.h index 544aa6b766a4..6bd5f33d9533 100644 --- a/security/apparmor/include/procattr.h +++ b/security/apparmor/include/procattr.h @@ -21,6 +21,5 @@ int aa_getprocattr(struct aa_profile *profile, char **string); int aa_setprocattr_changehat(char *args, size_t size, int test); int aa_setprocattr_changeprofile(char *fqname, bool onexec, int test); -int aa_setprocattr_permipc(char *fqname); #endif /* __AA_PROCATTR_H */ diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index 0f61dadca9e6..ed7e3aadba3a 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -572,8 +572,6 @@ static int apparmor_setprocattr(struct task_struct *task, char *name, } else if (strcmp(command, "permprofile") == 0) { error = aa_setprocattr_changeprofile(args, !AA_ONEXEC, AA_DO_TEST); - } else if (strcmp(command, "permipc") == 0) { - error = aa_setprocattr_permipc(args); } else { struct common_audit_data sa; struct apparmor_audit_data aad = {0,}; diff --git a/security/apparmor/procattr.c b/security/apparmor/procattr.c index 1b41c542d376..6c9390179b89 100644 --- a/security/apparmor/procattr.c +++ b/security/apparmor/procattr.c @@ -163,9 +163,3 @@ int aa_setprocattr_changeprofile(char *fqname, bool onexec, int test) name = aa_split_fqname(fqname, &ns_name); return aa_change_profile(ns_name, name, onexec, test); } - -int aa_setprocattr_permipc(char *fqname) -{ - /* TODO: add ipc permission querying */ - return -ENOTSUPP; -} -- cgit v1.2.3-59-g8ed1b From cf47aede3b9e197d3b4a028e2157bf7736665ac4 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Mon, 18 Feb 2013 16:07:34 -0800 Subject: apparmor: relax the restrictions on setting rlimits Instead of limiting the setting of the processes limits to current, relax this to tasks confined by the same profile, as the apparmor controls for rlimits are at a profile level granularity. Signed-off-by: John Johansen Acked-by: Steve Beattie --- security/apparmor/resource.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) (limited to 'security') diff --git a/security/apparmor/resource.c b/security/apparmor/resource.c index e1f3d7ef2c54..748bf0ca6c9f 100644 --- a/security/apparmor/resource.c +++ b/security/apparmor/resource.c @@ -15,6 +15,7 @@ #include #include "include/audit.h" +#include "include/context.h" #include "include/resource.h" #include "include/policy.h" @@ -90,17 +91,25 @@ int aa_map_resource(int resource) int aa_task_setrlimit(struct aa_profile *profile, struct task_struct *task, unsigned int resource, struct rlimit *new_rlim) { + struct aa_profile *task_profile; int error = 0; + rcu_read_lock(); + task_profile = aa_get_profile(aa_cred_profile(__task_cred(task))); + rcu_read_unlock(); + /* TODO: extend resource control to handle other (non current) - * processes. AppArmor rules currently have the implicit assumption - * that the task is setting the resource of the current process + * profiles. AppArmor rules currently have the implicit assumption + * that the task is setting the resource of a task confined with + * the same profile. */ - if ((task != current->group_leader) || + if (profile != task_profile || (profile->rlimits.mask & (1 << resource) && new_rlim->rlim_max > profile->rlimits.limits[resource].rlim_max)) error = -EACCES; + aa_put_profile(task_profile); + return audit_resource(profile, resource, new_rlim->rlim_max, error); } -- cgit v1.2.3-59-g8ed1b From 8e4ff109d0d2194d98e9e16325bb4102f6463b43 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Mon, 18 Feb 2013 16:08:34 -0800 Subject: apparmor: misc cleanup of match tidying up comments, includes and defines Signed-off-by: John Johansen Acked-by: Kees Cook --- security/apparmor/include/match.h | 19 +++++++++++++------ security/apparmor/match.c | 3 +-- 2 files changed, 14 insertions(+), 8 deletions(-) (limited to 'security') diff --git a/security/apparmor/include/match.h b/security/apparmor/include/match.h index 775843e7f984..bbbf56f5ba78 100644 --- a/security/apparmor/include/match.h +++ b/security/apparmor/include/match.h @@ -4,7 +4,7 @@ * This file contains AppArmor policy dfa matching engine definitions. * * Copyright (C) 1998-2008 Novell/SUSE - * Copyright 2009-2010 Canonical Ltd. + * Copyright 2009-2012 Canonical Ltd. * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License as @@ -16,7 +16,6 @@ #define __AA_MATCH_H #include -#include #define DFA_NOMATCH 0 #define DFA_START 1 @@ -29,12 +28,20 @@ * file format (--tables-file option; see Table File Format in the flex * info pages and the flex sources for documentation). The magic number * used in the header is 0x1B5E783D instead of 0xF13C57B1 though, because - * the YY_ID_CHK (check) and YY_ID_DEF (default) tables are used - * slightly differently (see the apparmor-parser package). + * new tables have been defined and others YY_ID_CHK (check) and YY_ID_DEF + * (default) tables are used slightly differently (see the apparmor-parser + * package). + * + * + * The data in the packed dfa is stored in network byte order, and the tables + * are arranged for flexibility. We convert the table data to host native + * byte order. + * + * The dfa begins with a table set header, and is followed by the actual + * tables. */ #define YYTH_MAGIC 0x1B5E783D -#define YYTH_DEF_RECURSE 0x1 /* DEF Table is recursive */ struct table_set_header { u32 th_magic; /* YYTH_MAGIC */ @@ -63,7 +70,7 @@ struct table_set_header { #define YYTD_DATA32 4 #define YYTD_DATA64 8 -/* Each ACCEPT2 table gets 6 dedicated flags, YYTD_DATAX define the +/* ACCEPT & ACCEPT2 tables gets 6 dedicated flags, YYTD_DATAX define the * first flags */ #define ACCEPT1_FLAGS(X) ((X) & 0x3f) diff --git a/security/apparmor/match.c b/security/apparmor/match.c index dfd25a9c9a69..1ff823031c73 100644 --- a/security/apparmor/match.c +++ b/security/apparmor/match.c @@ -4,7 +4,7 @@ * This file contains AppArmor dfa based regular expression matching engine * * Copyright (C) 1998-2008 Novell/SUSE - * Copyright 2009-2010 Canonical Ltd. + * Copyright 2009-2012 Canonical Ltd. * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License as @@ -137,7 +137,6 @@ static int verify_dfa(struct aa_dfa *dfa, int flags) for (i = 0; i < state_count; i++) { if (DEFAULT_TABLE(dfa)[i] >= state_count) goto out; - /* TODO: do check that DEF state recursion terminates */ if (BASE_TABLE(dfa)[i] + 255 >= trans_count) { printk(KERN_ERR "AppArmor DFA next/check upper " "bounds error\n"); -- cgit v1.2.3-59-g8ed1b From 180a6f5965a49535a7704c07691a6d1209904971 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Mon, 18 Feb 2013 16:09:34 -0800 Subject: apparmor: move perm defines into policy_unpack Signed-off-by: John Johansen Acked-by: Steve Beattie --- security/apparmor/include/match.h | 2 -- security/apparmor/policy_unpack.c | 3 +++ 2 files changed, 3 insertions(+), 2 deletions(-) (limited to 'security') diff --git a/security/apparmor/include/match.h b/security/apparmor/include/match.h index bbbf56f5ba78..001c43aa0406 100644 --- a/security/apparmor/include/match.h +++ b/security/apparmor/include/match.h @@ -20,8 +20,6 @@ #define DFA_NOMATCH 0 #define DFA_START 1 -#define DFA_VALID_PERM_MASK 0xffffffff -#define DFA_VALID_PERM2_MASK 0xffffffff /** * The format used for transition tables is based on the GNU flex table diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c index 329b1fd30749..ca48a7d8d5b3 100644 --- a/security/apparmor/policy_unpack.c +++ b/security/apparmor/policy_unpack.c @@ -290,6 +290,9 @@ static int unpack_strdup(struct aa_ext *e, char **string, const char *name) return res; } +#define DFA_VALID_PERM_MASK 0xffffffff +#define DFA_VALID_PERM2_MASK 0xffffffff + /** * verify_accept - verify the accept tables of a dfa * @dfa: dfa to verify accept tables of (NOT NULL) -- cgit v1.2.3-59-g8ed1b From a4987857d2c958b93b2faafe0811eea1a63ff59a Mon Sep 17 00:00:00 2001 From: John Johansen Date: Mon, 18 Feb 2013 16:10:34 -0800 Subject: apparmor: remove sid from profiles The sid is not going to be a direct property of a profile anymore, instead it will be directly related to the label, and the profile will pickup a label back reference. For null-profiles replace the use of sid with a per namespace unique id. Signed-off-by: John Johansen Acked-by: Kees Cook --- security/apparmor/include/policy.h | 4 ++-- security/apparmor/include/sid.h | 4 +++- security/apparmor/policy.c | 23 ++++++----------------- security/apparmor/policy_unpack.c | 1 - 4 files changed, 11 insertions(+), 21 deletions(-) (limited to 'security') diff --git a/security/apparmor/include/policy.h b/security/apparmor/include/policy.h index 95979c431e26..b25491a3046a 100644 --- a/security/apparmor/include/policy.h +++ b/security/apparmor/include/policy.h @@ -105,6 +105,7 @@ struct aa_ns_acct { * @acct: accounting for the namespace * @unconfined: special unconfined profile for the namespace * @sub_ns: list of namespaces under the current namespace. + * @uniq_null: uniq value used for null learning profiles * * An aa_namespace defines the set profiles that are searched to determine * which profile to attach to a task. Profiles can not be shared between @@ -127,6 +128,7 @@ struct aa_namespace { struct aa_ns_acct acct; struct aa_profile *unconfined; struct list_head sub_ns; + atomic_t uniq_null; }; /* struct aa_policydb - match engine for a policy @@ -148,7 +150,6 @@ struct aa_policydb { * @rename: optional profile name that this profile renamed * @xmatch: optional extended matching for unconfined executables names * @xmatch_len: xmatch prefix len, used to determine xmatch priority - * @sid: the unique security id number of this profile * @audit: the auditing mode of the profile * @mode: the enforcement mode of the profile * @flags: flags controlling profile behavior @@ -184,7 +185,6 @@ struct aa_profile { struct aa_dfa *xmatch; int xmatch_len; - u32 sid; enum audit_mode audit; enum profile_mode mode; u32 flags; diff --git a/security/apparmor/include/sid.h b/security/apparmor/include/sid.h index 020db35c3010..513ca0e48965 100644 --- a/security/apparmor/include/sid.h +++ b/security/apparmor/include/sid.h @@ -16,7 +16,9 @@ #include -struct aa_profile; +/* sid value that will not be allocated */ +#define AA_SID_INVALID 0 +#define AA_SID_ALLOC AA_SID_INVALID u32 aa_alloc_sid(void); void aa_free_sid(u32 sid); diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c index 813200384d97..13fc9efddd5d 100644 --- a/security/apparmor/policy.c +++ b/security/apparmor/policy.c @@ -87,7 +87,6 @@ #include "include/policy.h" #include "include/policy_unpack.h" #include "include/resource.h" -#include "include/sid.h" /* root profile namespace */ @@ -292,7 +291,6 @@ static struct aa_namespace *alloc_namespace(const char *prefix, if (!ns->unconfined) goto fail_unconfined; - ns->unconfined->sid = aa_alloc_sid(); ns->unconfined->flags = PFLAG_UNCONFINED | PFLAG_IX_ON_NAME_ERROR | PFLAG_IMMUTABLE; @@ -303,6 +301,8 @@ static struct aa_namespace *alloc_namespace(const char *prefix, */ ns->unconfined->ns = aa_get_namespace(ns); + atomic_set(&ns->uniq_null, 0); + return ns; fail_unconfined: @@ -497,7 +497,6 @@ static void __replace_profile(struct aa_profile *old, struct aa_profile *new) /* released when @new is freed */ new->parent = aa_get_profile(old->parent); new->ns = aa_get_namespace(old->ns); - new->sid = old->sid; __list_add_profile(&policy->profiles, new); /* inherit children */ list_for_each_entry_safe(child, tmp, &old->base.profiles, base.list) { @@ -665,7 +664,7 @@ struct aa_profile *aa_alloc_profile(const char *hname) * @hat: true if the null- learning profile is a hat * * Create a null- complain mode profile used in learning mode. The name of - * the profile is unique and follows the format of parent//null-sid. + * the profile is unique and follows the format of parent//null-. * * null profiles are added to the profile list but the list does not * hold a count on them so that they are automatically released when @@ -677,20 +676,19 @@ struct aa_profile *aa_new_null_profile(struct aa_profile *parent, int hat) { struct aa_profile *profile = NULL; char *name; - u32 sid = aa_alloc_sid(); + int uniq = atomic_inc_return(&parent->ns->uniq_null); /* freed below */ name = kmalloc(strlen(parent->base.hname) + 2 + 7 + 8, GFP_KERNEL); if (!name) goto fail; - sprintf(name, "%s//null-%x", parent->base.hname, sid); + sprintf(name, "%s//null-%x", parent->base.hname, uniq); profile = aa_alloc_profile(name); kfree(name); if (!profile) goto fail; - profile->sid = sid; profile->mode = APPARMOR_COMPLAIN; profile->flags = PFLAG_NULL; if (hat) @@ -708,7 +706,6 @@ struct aa_profile *aa_new_null_profile(struct aa_profile *parent, int hat) return profile; fail: - aa_free_sid(sid); return NULL; } @@ -749,7 +746,6 @@ static void free_profile(struct aa_profile *profile) aa_free_cap_rules(&profile->caps); aa_free_rlimit_rules(&profile->rlimits); - aa_free_sid(profile->sid); aa_put_dfa(profile->xmatch); aa_put_dfa(profile->policy.dfa); @@ -972,7 +968,6 @@ static void __add_new_profile(struct aa_namespace *ns, struct aa_policy *policy, profile->parent = aa_get_profile((struct aa_profile *) policy); __list_add_profile(&policy->profiles, profile); /* released on free_profile */ - profile->sid = aa_alloc_sid(); profile->ns = aa_get_namespace(ns); } @@ -1110,14 +1105,8 @@ audit: if (!error) { if (rename_profile) __replace_profile(rename_profile, new_profile); - if (old_profile) { - /* when there are both rename and old profiles - * inherit old profiles sid - */ - if (rename_profile) - aa_free_sid(new_profile->sid); + if (old_profile) __replace_profile(old_profile, new_profile); - } if (!(old_profile || rename_profile)) __add_new_profile(ns, policy, new_profile); } diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c index ca48a7d8d5b3..6dac7d77cb4d 100644 --- a/security/apparmor/policy_unpack.c +++ b/security/apparmor/policy_unpack.c @@ -27,7 +27,6 @@ #include "include/match.h" #include "include/policy.h" #include "include/policy_unpack.h" -#include "include/sid.h" /* * The AppArmor interface treats data as a type byte followed by the -- cgit v1.2.3-59-g8ed1b From 4da05cc08da3f2058cecbe42ed9f4803d669730a Mon Sep 17 00:00:00 2001 From: John Johansen Date: Mon, 18 Feb 2013 16:11:34 -0800 Subject: apparmor: move the free_profile fn ahead of aa_alloc_profile Move the free_profile fn ahead of aa_alloc_profile so it can be used in aa_alloc_profile without a forward declaration. Signed-off-by: John Johansen Acked-by: Kees Cook --- security/apparmor/policy.c | 150 ++++++++++++++++++++++----------------------- 1 file changed, 75 insertions(+), 75 deletions(-) (limited to 'security') diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c index 13fc9efddd5d..f4ee72b44de4 100644 --- a/security/apparmor/policy.c +++ b/security/apparmor/policy.c @@ -634,81 +634,6 @@ void __init aa_free_root_ns(void) aa_put_namespace(ns); } -/** - * aa_alloc_profile - allocate, initialize and return a new profile - * @hname: name of the profile (NOT NULL) - * - * Returns: refcount profile or NULL on failure - */ -struct aa_profile *aa_alloc_profile(const char *hname) -{ - struct aa_profile *profile; - - /* freed by free_profile - usually through aa_put_profile */ - profile = kzalloc(sizeof(*profile), GFP_KERNEL); - if (!profile) - return NULL; - - if (!policy_init(&profile->base, NULL, hname)) { - kzfree(profile); - return NULL; - } - - /* refcount released by caller */ - return profile; -} - -/** - * aa_new_null_profile - create a new null-X learning profile - * @parent: profile that caused this profile to be created (NOT NULL) - * @hat: true if the null- learning profile is a hat - * - * Create a null- complain mode profile used in learning mode. The name of - * the profile is unique and follows the format of parent//null-. - * - * null profiles are added to the profile list but the list does not - * hold a count on them so that they are automatically released when - * not in use. - * - * Returns: new refcounted profile else NULL on failure - */ -struct aa_profile *aa_new_null_profile(struct aa_profile *parent, int hat) -{ - struct aa_profile *profile = NULL; - char *name; - int uniq = atomic_inc_return(&parent->ns->uniq_null); - - /* freed below */ - name = kmalloc(strlen(parent->base.hname) + 2 + 7 + 8, GFP_KERNEL); - if (!name) - goto fail; - sprintf(name, "%s//null-%x", parent->base.hname, uniq); - - profile = aa_alloc_profile(name); - kfree(name); - if (!profile) - goto fail; - - profile->mode = APPARMOR_COMPLAIN; - profile->flags = PFLAG_NULL; - if (hat) - profile->flags |= PFLAG_HAT; - - /* released on free_profile */ - profile->parent = aa_get_profile(parent); - profile->ns = aa_get_namespace(parent->ns); - - write_lock(&profile->ns->lock); - __list_add_profile(&parent->base.profiles, profile); - write_unlock(&profile->ns->lock); - - /* refcount released by caller */ - return profile; - -fail: - return NULL; -} - /** * free_profile - free a profile * @profile: the profile to free (MAYBE NULL) @@ -786,6 +711,81 @@ void aa_free_profile_kref(struct kref *kref) free_profile(p); } +/** + * aa_alloc_profile - allocate, initialize and return a new profile + * @hname: name of the profile (NOT NULL) + * + * Returns: refcount profile or NULL on failure + */ +struct aa_profile *aa_alloc_profile(const char *hname) +{ + struct aa_profile *profile; + + /* freed by free_profile - usually through aa_put_profile */ + profile = kzalloc(sizeof(*profile), GFP_KERNEL); + if (!profile) + return NULL; + + if (!policy_init(&profile->base, NULL, hname)) { + kzfree(profile); + return NULL; + } + + /* refcount released by caller */ + return profile; +} + +/** + * aa_new_null_profile - create a new null-X learning profile + * @parent: profile that caused this profile to be created (NOT NULL) + * @hat: true if the null- learning profile is a hat + * + * Create a null- complain mode profile used in learning mode. The name of + * the profile is unique and follows the format of parent//null-. + * + * null profiles are added to the profile list but the list does not + * hold a count on them so that they are automatically released when + * not in use. + * + * Returns: new refcounted profile else NULL on failure + */ +struct aa_profile *aa_new_null_profile(struct aa_profile *parent, int hat) +{ + struct aa_profile *profile = NULL; + char *name; + int uniq = atomic_inc_return(&parent->ns->uniq_null); + + /* freed below */ + name = kmalloc(strlen(parent->base.hname) + 2 + 7 + 8, GFP_KERNEL); + if (!name) + goto fail; + sprintf(name, "%s//null-%x", parent->base.hname, uniq); + + profile = aa_alloc_profile(name); + kfree(name); + if (!profile) + goto fail; + + profile->mode = APPARMOR_COMPLAIN; + profile->flags = PFLAG_NULL; + if (hat) + profile->flags |= PFLAG_HAT; + + /* released on free_profile */ + profile->parent = aa_get_profile(parent); + profile->ns = aa_get_namespace(parent->ns); + + write_lock(&profile->ns->lock); + __list_add_profile(&parent->base.profiles, profile); + write_unlock(&profile->ns->lock); + + /* refcount released by caller */ + return profile; + +fail: + return NULL; +} + /* TODO: profile accounting - setup in remove */ /** -- cgit v1.2.3-59-g8ed1b From ed686308c6837ff67f56e4115d0fd6bdc65a4313 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Mon, 18 Feb 2013 16:12:34 -0800 Subject: apparmor: reserve and mask off the top 8 bits of the base field The top 8 bits of the base field have never been used, in fact can't be used, by the current 'dfa16' format. However they will be used in the future as flags, so mask them off when using base as an index value. Note: the use of the top 8 bits, without masking is trapped by the verify checks that base entries are within the size bounds. Signed-off-by: John Johansen Acked-by: Kees Cook --- security/apparmor/match.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) (limited to 'security') diff --git a/security/apparmor/match.c b/security/apparmor/match.c index 1ff823031c73..727eb4200d5c 100644 --- a/security/apparmor/match.c +++ b/security/apparmor/match.c @@ -23,6 +23,8 @@ #include "include/apparmor.h" #include "include/match.h" +#define base_idx(X) ((X) & 0xffffff) + /** * unpack_table - unpack a dfa table (one of accept, default, base, next check) * @blob: data to unpack (NOT NULL) @@ -137,7 +139,7 @@ static int verify_dfa(struct aa_dfa *dfa, int flags) for (i = 0; i < state_count; i++) { if (DEFAULT_TABLE(dfa)[i] >= state_count) goto out; - if (BASE_TABLE(dfa)[i] + 255 >= trans_count) { + if (base_idx(BASE_TABLE(dfa)[i]) + 255 >= trans_count) { printk(KERN_ERR "AppArmor DFA next/check upper " "bounds error\n"); goto out; @@ -313,7 +315,7 @@ unsigned int aa_dfa_match_len(struct aa_dfa *dfa, unsigned int start, u8 *equiv = EQUIV_TABLE(dfa); /* default is direct to next state */ for (; len; len--) { - pos = base[state] + equiv[(u8) *str++]; + pos = base_idx(base[state]) + equiv[(u8) *str++]; if (check[pos] == state) state = next[pos]; else @@ -322,7 +324,7 @@ unsigned int aa_dfa_match_len(struct aa_dfa *dfa, unsigned int start, } else { /* default is direct to next state */ for (; len; len--) { - pos = base[state] + (u8) *str++; + pos = base_idx(base[state]) + (u8) *str++; if (check[pos] == state) state = next[pos]; else @@ -363,7 +365,7 @@ unsigned int aa_dfa_match(struct aa_dfa *dfa, unsigned int start, u8 *equiv = EQUIV_TABLE(dfa); /* default is direct to next state */ while (*str) { - pos = base[state] + equiv[(u8) *str++]; + pos = base_idx(base[state]) + equiv[(u8) *str++]; if (check[pos] == state) state = next[pos]; else @@ -372,7 +374,7 @@ unsigned int aa_dfa_match(struct aa_dfa *dfa, unsigned int start, } else { /* default is direct to next state */ while (*str) { - pos = base[state] + (u8) *str++; + pos = base_idx(base[state]) + (u8) *str++; if (check[pos] == state) state = next[pos]; else @@ -408,14 +410,14 @@ unsigned int aa_dfa_next(struct aa_dfa *dfa, unsigned int state, u8 *equiv = EQUIV_TABLE(dfa); /* default is direct to next state */ - pos = base[state] + equiv[(u8) c]; + pos = base_idx(base[state]) + equiv[(u8) c]; if (check[pos] == state) state = next[pos]; else state = def[state]; } else { /* default is direct to next state */ - pos = base[state] + (u8) c; + pos = base_idx(base[state]) + (u8) c; if (check[pos] == state) state = next[pos]; else -- cgit v1.2.3-59-g8ed1b From b492d50bf597b87ab7ea1e738ec837f74b11594e Mon Sep 17 00:00:00 2001 From: John Johansen Date: Mon, 18 Feb 2013 16:13:34 -0800 Subject: apparmor: fix the audit type table The audit type table is missing a comma so that KILLED comes out as KILLEDAUTO. Signed-off-by: John Johansen Acked-by: Steve Beattie --- security/apparmor/audit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security') diff --git a/security/apparmor/audit.c b/security/apparmor/audit.c index 3ae28db5a64f..031d2d9dd695 100644 --- a/security/apparmor/audit.c +++ b/security/apparmor/audit.c @@ -88,7 +88,7 @@ static const char *const aa_audit_type[] = { "HINT", "STATUS", "ERROR", - "KILLED" + "KILLED", "AUTO" }; -- cgit v1.2.3-59-g8ed1b From 41d1b3e868c263e8b43dd5903a70633e05ae58a6 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Thu, 21 Feb 2013 01:14:17 -0800 Subject: apparmor: Fix smatch warning in aa_remove_profiles smatch reports error: potential NULL dereference 'ns'. this can not actually occur because it relies on aa_split_fqname setting both ns_name and name as null but ns_name will actually always have a value in this case. so remove the unnecessary if (ns_name) conditional that is resulting in the false positive further down. Signed-off-by: John Johansen --- security/apparmor/policy.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) (limited to 'security') diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c index f4ee72b44de4..0f345c4dee5f 100644 --- a/security/apparmor/policy.c +++ b/security/apparmor/policy.c @@ -1156,14 +1156,12 @@ ssize_t aa_remove_profiles(char *fqname, size_t size) if (fqname[0] == ':') { char *ns_name; name = aa_split_fqname(fqname, &ns_name); - if (ns_name) { - /* released below */ - ns = aa_find_namespace(root, ns_name); - if (!ns) { - info = "namespace does not exist"; - error = -ENOENT; - goto fail; - } + /* released below */ + ns = aa_find_namespace(root, ns_name); + if (!ns) { + info = "namespace does not exist"; + error = -ENOENT; + goto fail; } } else /* released below */ -- cgit v1.2.3-59-g8ed1b From 53fe8b9961716033571d9799005bfdbbafa5162c Mon Sep 17 00:00:00 2001 From: John Johansen Date: Thu, 21 Feb 2013 13:25:44 -0800 Subject: apparmor: fix sparse warnings Fix a couple of warning reported by sparse Signed-off-by: John Johansen --- security/apparmor/include/file.h | 14 +++++++------- security/apparmor/lsm.c | 8 ++++---- 2 files changed, 11 insertions(+), 11 deletions(-) (limited to 'security') diff --git a/security/apparmor/include/file.h b/security/apparmor/include/file.h index 967b2deda376..2c922b86bd44 100644 --- a/security/apparmor/include/file.h +++ b/security/apparmor/include/file.h @@ -186,11 +186,6 @@ static inline void aa_free_file_rules(struct aa_file_rules *rules) aa_free_domain_entries(&rules->trans); } -#define ACC_FMODE(x) (("\000\004\002\006"[(x)&O_ACCMODE]) | (((x) << 1) & 0x40)) - -/* from namei.c */ -#define MAP_OPEN_FLAGS(x) ((((x) + 1) & O_ACCMODE) ? (x) + 1 : (x)) - /** * aa_map_file_perms - map file flags to AppArmor permissions * @file: open file to map flags to AppArmor permissions @@ -199,8 +194,13 @@ static inline void aa_free_file_rules(struct aa_file_rules *rules) */ static inline u32 aa_map_file_to_perms(struct file *file) { - int flags = MAP_OPEN_FLAGS(file->f_flags); - u32 perms = ACC_FMODE(file->f_mode); + int flags = file->f_flags; + u32 perms = 0; + + if (file->f_mode & FMODE_WRITE) + perms |= MAY_WRITE; + if (file->f_mode & FMODE_READ) + perms |= MAY_READ; if ((flags & O_APPEND) && (perms & MAY_WRITE)) perms = (perms & ~MAY_WRITE) | MAY_APPEND; diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index ed7e3aadba3a..10843aa5a368 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -909,8 +909,11 @@ static int __init apparmor_init(void) error = register_security(&apparmor_ops); if (error) { + struct cred *cred = (struct cred *)current->real_cred; + aa_free_task_context(cred->security); + cred->security = NULL; AA_ERROR("Unable to register AppArmor\n"); - goto set_init_cxt_out; + goto register_security_out; } /* Report that AppArmor successfully initialized */ @@ -924,9 +927,6 @@ static int __init apparmor_init(void) return error; -set_init_cxt_out: - aa_free_task_context(current->real_cred->security); - register_security_out: aa_free_root_ns(); -- cgit v1.2.3-59-g8ed1b From 214beacaa7b669473bc963af719fa359a8312ea4 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Wed, 27 Feb 2013 03:43:40 -0800 Subject: apparmor: localize getting the security context to a few macros Signed-off-by: John Johansen Acked-by: Seth Arnold --- security/apparmor/context.c | 10 +++++----- security/apparmor/domain.c | 6 +++--- security/apparmor/include/context.h | 7 +++++-- security/apparmor/lsm.c | 22 +++++++++++----------- 4 files changed, 24 insertions(+), 21 deletions(-) (limited to 'security') diff --git a/security/apparmor/context.c b/security/apparmor/context.c index 3f911afa2bb9..d5af1d15f26d 100644 --- a/security/apparmor/context.c +++ b/security/apparmor/context.c @@ -93,7 +93,7 @@ struct aa_profile *aa_get_task_profile(struct task_struct *task) */ int aa_replace_current_profile(struct aa_profile *profile) { - struct aa_task_cxt *cxt = current_cred()->security; + struct aa_task_cxt *cxt = current_cxt(); struct cred *new; BUG_ON(!profile); @@ -104,7 +104,7 @@ int aa_replace_current_profile(struct aa_profile *profile) if (!new) return -ENOMEM; - cxt = new->security; + cxt = cred_cxt(new); if (unconfined(profile) || (cxt->profile->ns != profile->ns)) /* if switching to unconfined or a different profile namespace * clear out context state @@ -136,7 +136,7 @@ int aa_set_current_onexec(struct aa_profile *profile) if (!new) return -ENOMEM; - cxt = new->security; + cxt = cred_cxt(new); aa_get_profile(profile); aa_put_profile(cxt->onexec); cxt->onexec = profile; @@ -163,7 +163,7 @@ int aa_set_current_hat(struct aa_profile *profile, u64 token) return -ENOMEM; BUG_ON(!profile); - cxt = new->security; + cxt = cred_cxt(new); if (!cxt->previous) { /* transfer refcount */ cxt->previous = cxt->profile; @@ -200,7 +200,7 @@ int aa_restore_previous_profile(u64 token) if (!new) return -ENOMEM; - cxt = new->security; + cxt = cred_cxt(new); if (cxt->token != token) { abort_creds(new); return -EACCES; diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index 07fcb09b990f..01b7bd669a88 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -356,7 +356,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm) if (bprm->cred_prepared) return 0; - cxt = bprm->cred->security; + cxt = cred_cxt(bprm->cred); BUG_ON(!cxt); profile = aa_get_profile(aa_newest_version(cxt->profile)); @@ -551,7 +551,7 @@ int apparmor_bprm_secureexec(struct linux_binprm *bprm) void apparmor_bprm_committing_creds(struct linux_binprm *bprm) { struct aa_profile *profile = __aa_current_profile(); - struct aa_task_cxt *new_cxt = bprm->cred->security; + struct aa_task_cxt *new_cxt = cred_cxt(bprm->cred); /* bail out if unconfined or not changing profile */ if ((new_cxt->profile == profile) || @@ -628,7 +628,7 @@ int aa_change_hat(const char *hats[], int count, u64 token, bool permtest) /* released below */ cred = get_current_cred(); - cxt = cred->security; + cxt = cred_cxt(cred); profile = aa_cred_profile(cred); previous_profile = cxt->previous; diff --git a/security/apparmor/include/context.h b/security/apparmor/include/context.h index 4cecad313227..d44ba5802e3d 100644 --- a/security/apparmor/include/context.h +++ b/security/apparmor/include/context.h @@ -21,6 +21,9 @@ #include "policy.h" +#define cred_cxt(X) (X)->security +#define current_cxt() cred_cxt(current_cred()) + /* struct aa_file_cxt - the AppArmor context the file was opened in * @perms: the permission the file was opened with * @@ -93,7 +96,7 @@ struct aa_profile *aa_get_task_profile(struct task_struct *task); */ static inline struct aa_profile *aa_cred_profile(const struct cred *cred) { - struct aa_task_cxt *cxt = cred->security; + struct aa_task_cxt *cxt = cred_cxt(cred); BUG_ON(!cxt || !cxt->profile); return aa_newest_version(cxt->profile); } @@ -145,7 +148,7 @@ static inline struct aa_profile *__aa_current_profile(void) */ static inline struct aa_profile *aa_current_profile(void) { - const struct aa_task_cxt *cxt = current_cred()->security; + const struct aa_task_cxt *cxt = current_cxt(); struct aa_profile *profile; BUG_ON(!cxt || !cxt->profile); diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index 10843aa5a368..2027fdf2060b 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -48,8 +48,8 @@ int apparmor_initialized __initdata; */ static void apparmor_cred_free(struct cred *cred) { - aa_free_task_context(cred->security); - cred->security = NULL; + aa_free_task_context(cred_cxt(cred)); + cred_cxt(cred) = NULL; } /* @@ -62,7 +62,7 @@ static int apparmor_cred_alloc_blank(struct cred *cred, gfp_t gfp) if (!cxt) return -ENOMEM; - cred->security = cxt; + cred_cxt(cred) = cxt; return 0; } @@ -77,8 +77,8 @@ static int apparmor_cred_prepare(struct cred *new, const struct cred *old, if (!cxt) return -ENOMEM; - aa_dup_task_context(cxt, old->security); - new->security = cxt; + aa_dup_task_context(cxt, cred_cxt(old)); + cred_cxt(new) = cxt; return 0; } @@ -87,8 +87,8 @@ 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_task_cxt *old_cxt = old->security; - struct aa_task_cxt *new_cxt = new->security; + const struct aa_task_cxt *old_cxt = cred_cxt(old); + struct aa_task_cxt *new_cxt = cred_cxt(new); aa_dup_task_context(new_cxt, old_cxt); } @@ -507,7 +507,7 @@ static int apparmor_getprocattr(struct task_struct *task, char *name, int error = -ENOENT; /* released below */ const struct cred *cred = get_task_cred(task); - struct aa_task_cxt *cxt = cred->security; + struct aa_task_cxt *cxt = cred_cxt(cred); if (strcmp(name, "current") == 0) error = aa_getprocattr(aa_newest_version(cxt->profile), @@ -880,7 +880,7 @@ static int __init set_init_cxt(void) return -ENOMEM; cxt->profile = aa_get_profile(root_ns->unconfined); - cred->security = cxt; + cred_cxt(cred) = cxt; return 0; } @@ -910,8 +910,8 @@ static int __init apparmor_init(void) error = register_security(&apparmor_ops); if (error) { struct cred *cred = (struct cred *)current->real_cred; - aa_free_task_context(cred->security); - cred->security = NULL; + aa_free_task_context(cred_cxt(cred)); + cred_cxt(cred) = NULL; AA_ERROR("Unable to register AppArmor\n"); goto register_security_out; } -- cgit v1.2.3-59-g8ed1b From 3eea57c26e49a5add4c053a031cc2a1977b7c48e Mon Sep 17 00:00:00 2001 From: John Johansen Date: Wed, 27 Feb 2013 03:44:40 -0800 Subject: apparmor: fix setprocattr arg processing for onexec the exec file isn't processing its command arg. It should only set be responding to a command of exec. Also cleanup setprocattr some more while we are at it. Signed-off-by: John Johansen --- security/apparmor/lsm.c | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) (limited to 'security') diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index 2027fdf2060b..2e2a0dd4a73f 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -529,6 +529,8 @@ static int apparmor_getprocattr(struct task_struct *task, char *name, static int apparmor_setprocattr(struct task_struct *task, char *name, void *value, size_t size) { + struct common_audit_data sa; + struct apparmor_audit_data aad = {0,}; char *command, *args = value; size_t arg_size; int error; @@ -572,28 +574,31 @@ static int apparmor_setprocattr(struct task_struct *task, char *name, } else if (strcmp(command, "permprofile") == 0) { error = aa_setprocattr_changeprofile(args, !AA_ONEXEC, AA_DO_TEST); - } else { - struct common_audit_data sa; - struct apparmor_audit_data aad = {0,}; - sa.type = LSM_AUDIT_DATA_NONE; - sa.aad = &aad; - aad.op = OP_SETPROCATTR; - aad.info = name; - aad.error = -EINVAL; - return aa_audit(AUDIT_APPARMOR_DENIED, - __aa_current_profile(), GFP_KERNEL, - &sa, NULL); - } + } else + goto fail; } else if (strcmp(name, "exec") == 0) { - error = aa_setprocattr_changeprofile(args, AA_ONEXEC, - !AA_DO_TEST); - } else { + if (strcmp(command, "exec") == 0) + error = aa_setprocattr_changeprofile(args, AA_ONEXEC, + !AA_DO_TEST); + else + goto fail; + } else /* only support the "current" and "exec" process attributes */ return -EINVAL; - } + if (!error) error = size; return error; + +fail: + sa.type = LSM_AUDIT_DATA_NONE; + sa.aad = &aad; + aad.profile = aa_current_profile(); + aad.op = OP_SETPROCATTR; + aad.info = name; + aad.error = -EINVAL; + aa_audit_msg(AUDIT_APPARMOR_DENIED, &sa, NULL); + return -EINVAL; } static int apparmor_task_setrlimit(struct task_struct *task, -- cgit v1.2.3-59-g8ed1b From 2654bfbc2bd0e1e64f0b257c21da23f6cec32c6c Mon Sep 17 00:00:00 2001 From: John Johansen Date: Wed, 27 Feb 2013 03:45:05 -0800 Subject: apparmor: fix fully qualified name parsing currently apparmor name parsing is only correctly handling :: but ::// is also a valid form and what is exported to userspace. Signed-off-by: John Johansen --- security/apparmor/lib.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'security') diff --git a/security/apparmor/lib.c b/security/apparmor/lib.c index d6e1f2148398..d40bc592180d 100644 --- a/security/apparmor/lib.c +++ b/security/apparmor/lib.c @@ -45,8 +45,10 @@ char *aa_split_fqname(char *fqname, char **ns_name) *ns_name = skip_spaces(&name[1]); if (split) { /* overwrite ':' with \0 */ - *split = 0; - name = skip_spaces(split + 1); + *split++ = 0; + if (strncmp(split, "//", 2) == 0) + split += 2; + name = skip_spaces(split); } else /* a ns name without a following profile is allowed */ name = NULL; -- cgit v1.2.3-59-g8ed1b From b5b3ee6c9cca8b6e1aa8c757e570f08f802c5573 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Mon, 6 May 2013 03:10:35 +0100 Subject: apparmor: no need to delay vfree() vfree() can be called from interrupt contexts now Signed-off-by: Al Viro Acked-by: John Johansen Signed-off-by: James Morris --- security/apparmor/lib.c | 24 +++--------------------- 1 file changed, 3 insertions(+), 21 deletions(-) (limited to 'security') diff --git a/security/apparmor/lib.c b/security/apparmor/lib.c index d40bc592180d..fcfe0233574c 100644 --- a/security/apparmor/lib.c +++ b/security/apparmor/lib.c @@ -110,19 +110,6 @@ void *__aa_kvmalloc(size_t size, gfp_t flags) return buffer; } -/** - * do_vfree - workqueue routine for freeing vmalloced memory - * @work: data to be freed - * - * The work_struct is overlaid to the data being freed, as at the point - * the work is scheduled the data is no longer valid, be its freeing - * needs to be delayed until safe. - */ -static void do_vfree(struct work_struct *work) -{ - vfree(work); -} - /** * kvfree - free an allocation do by kvmalloc * @buffer: buffer to free (MAYBE_NULL) @@ -131,13 +118,8 @@ static void do_vfree(struct work_struct *work) */ void kvfree(void *buffer) { - if (is_vmalloc_addr(buffer)) { - /* Data is no longer valid so just use the allocated space - * as the work_struct - */ - struct work_struct *work = (struct work_struct *) buffer; - INIT_WORK(work, do_vfree); - schedule_work(work); - } else + if (is_vmalloc_addr(buffer)) + vfree(buffer); + else kfree(buffer); } -- cgit v1.2.3-59-g8ed1b From c673944347edfd4362b10eea11ac384a582b1cf5 Mon Sep 17 00:00:00 2001 From: Casey Schaufler Date: Wed, 22 May 2013 18:42:56 -0700 Subject: Smack: Local IPv6 port based controls Smack does not provide access controls on IPv6 communications. This patch introduces a mechanism for maintaining Smack lables for local IPv6 communications. It is based on labeling local ports. The behavior should be compatible with any future "real" IPv6 support as it provides no interfaces for users to manipulate the labeling. Remote IPv6 connections use the ambient label the same way that unlabeled IPv4 packets are treated. Targeted for git://git.gitorious.org/smack-next/kernel.git Signed-off-by: Casey Schaufler --- security/smack/smack.h | 11 ++ security/smack/smack_lsm.c | 348 +++++++++++++++++++++++++++++++++++++++------ 2 files changed, 319 insertions(+), 40 deletions(-) (limited to 'security') diff --git a/security/smack/smack.h b/security/smack/smack.h index 8ad30955e15d..bb28e099abfe 100644 --- a/security/smack/smack.h +++ b/security/smack/smack.h @@ -93,6 +93,17 @@ struct smk_netlbladdr { char *smk_label; /* label */ }; +/* + * An entry in the table identifying ports. + */ +struct smk_port_label { + struct list_head list; + struct sock *smk_sock; /* socket initialized on */ + unsigned short smk_port; /* the port number */ + char *smk_in; /* incoming label */ + char *smk_out; /* outgoing label */ +}; + /* * This is the repository for labels seen so that it is * not necessary to keep allocating tiny chuncks of memory diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index d52c780bdb78..609e89de3c24 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -27,10 +27,13 @@ #include #include #include +#include #include #include #include #include +#include +#include #include #include #include @@ -45,6 +48,12 @@ #define TRANS_TRUE "TRUE" #define TRANS_TRUE_SIZE 4 +#define SMK_CONNECTING 0 +#define SMK_RECEIVING 1 +#define SMK_SENDING 2 + +LIST_HEAD(smk_ipv6_port_list); + /** * smk_fetch - Fetch the smack label from a file. * @ip: a pointer to the inode @@ -1877,6 +1886,155 @@ static int smack_netlabel_send(struct sock *sk, struct sockaddr_in *sap) return smack_netlabel(sk, sk_lbl); } +/** + * smk_ipv6_port_label - Smack port access table management + * @sock: socket + * @address: address + * + * Create or update the port list entry + */ +static void smk_ipv6_port_label(struct socket *sock, struct sockaddr *address) +{ + struct sock *sk = sock->sk; + struct sockaddr_in6 *addr6; + struct socket_smack *ssp = sock->sk->sk_security; + struct smk_port_label *spp; + unsigned short port = 0; + + if (address == NULL) { + /* + * This operation is changing the Smack information + * on the bound socket. Take the changes to the port + * as well. + */ + list_for_each_entry(spp, &smk_ipv6_port_list, list) { + if (sk != spp->smk_sock) + continue; + spp->smk_in = ssp->smk_in; + spp->smk_out = ssp->smk_out; + return; + } + /* + * A NULL address is only used for updating existing + * bound entries. If there isn't one, it's OK. + */ + return; + } + + addr6 = (struct sockaddr_in6 *)address; + port = ntohs(addr6->sin6_port); + /* + * This is a special case that is safely ignored. + */ + if (port == 0) + return; + + /* + * Look for an existing port list entry. + * This is an indication that a port is getting reused. + */ + list_for_each_entry(spp, &smk_ipv6_port_list, list) { + if (spp->smk_port != port) + continue; + spp->smk_port = port; + spp->smk_sock = sk; + spp->smk_in = ssp->smk_in; + spp->smk_out = ssp->smk_out; + return; + } + + /* + * A new port entry is required. + */ + spp = kzalloc(sizeof(*spp), GFP_KERNEL); + if (spp == NULL) + return; + + spp->smk_port = port; + spp->smk_sock = sk; + spp->smk_in = ssp->smk_in; + spp->smk_out = ssp->smk_out; + + list_add(&spp->list, &smk_ipv6_port_list); + return; +} + +/** + * smk_ipv6_port_check - check Smack port access + * @sock: socket + * @address: address + * + * Create or update the port list entry + */ +static int smk_ipv6_port_check(struct sock *sk, struct sockaddr *address, + int act) +{ + __be16 *bep; + __be32 *be32p; + struct sockaddr_in6 *addr6; + struct smk_port_label *spp; + struct socket_smack *ssp = sk->sk_security; + unsigned short port = 0; + char *subject; + char *object; + struct smk_audit_info ad; +#ifdef CONFIG_AUDIT + struct lsm_network_audit net; +#endif + + if (act == SMK_RECEIVING) { + subject = smack_net_ambient; + object = ssp->smk_in; + } else { + subject = ssp->smk_out; + object = smack_net_ambient; + } + + /* + * Get the IP address and port from the address. + */ + addr6 = (struct sockaddr_in6 *)address; + port = ntohs(addr6->sin6_port); + bep = (__be16 *)(&addr6->sin6_addr); + be32p = (__be32 *)(&addr6->sin6_addr); + + /* + * It's remote, so port lookup does no good. + */ + if (be32p[0] || be32p[1] || be32p[2] || bep[6] || ntohs(bep[7]) != 1) + goto auditout; + + /* + * It's local so the send check has to have passed. + */ + if (act == SMK_RECEIVING) { + subject = smack_known_web.smk_known; + goto auditout; + } + + list_for_each_entry(spp, &smk_ipv6_port_list, list) { + if (spp->smk_port != port) + continue; + object = spp->smk_in; + if (act == SMK_CONNECTING) + ssp->smk_packet = spp->smk_out; + break; + } + +auditout: + +#ifdef CONFIG_AUDIT + smk_ad_init_net(&ad, __func__, LSM_AUDIT_DATA_NET, &net); + ad.a.u.net->family = sk->sk_family; + ad.a.u.net->dport = port; + if (act == SMK_RECEIVING) + ad.a.u.net->v6info.saddr = addr6->sin6_addr; + else + ad.a.u.net->v6info.daddr = addr6->sin6_addr; +#endif + return smk_access(subject, object, MAY_WRITE, &ad); +} + /** * smack_inode_setsecurity - set smack xattrs * @inode: the object @@ -1926,7 +2084,7 @@ static int smack_inode_setsecurity(struct inode *inode, const char *name, ssp->smk_in = sp; else if (strcmp(name, XATTR_SMACK_IPOUT) == 0) { ssp->smk_out = sp; - if (sock->sk->sk_family != PF_UNIX) { + if (sock->sk->sk_family == PF_INET) { rc = smack_netlabel(sock->sk, SMACK_CIPSO_SOCKET); if (rc != 0) printk(KERN_WARNING @@ -1936,6 +2094,9 @@ static int smack_inode_setsecurity(struct inode *inode, const char *name, } else return -EOPNOTSUPP; + if (sock->sk->sk_family == PF_INET6) + smk_ipv6_port_label(sock, NULL); + return 0; } @@ -1962,6 +2123,25 @@ static int smack_socket_post_create(struct socket *sock, int family, return smack_netlabel(sock->sk, SMACK_CIPSO_SOCKET); } +/** + * smack_socket_bind - record port binding information. + * @sock: the socket + * @address: the port address + * @addrlen: size of the address + * + * Records the label bound to a port. + * + * Returns 0 + */ +static int smack_socket_bind(struct socket *sock, struct sockaddr *address, + int addrlen) +{ + if (sock->sk != NULL && sock->sk->sk_family == PF_INET6) + smk_ipv6_port_label(sock, address); + + return 0; +} + /** * smack_socket_connect - connect access check * @sock: the socket @@ -1975,12 +2155,24 @@ static int smack_socket_post_create(struct socket *sock, int family, static int smack_socket_connect(struct socket *sock, struct sockaddr *sap, int addrlen) { - if (sock->sk == NULL || sock->sk->sk_family != PF_INET) + int rc = 0; + + if (sock->sk == NULL) return 0; - if (addrlen < sizeof(struct sockaddr_in)) - return -EINVAL; - return smack_netlabel_send(sock->sk, (struct sockaddr_in *)sap); + switch (sock->sk->sk_family) { + case PF_INET: + if (addrlen < sizeof(struct sockaddr_in)) + return -EINVAL; + rc = smack_netlabel_send(sock->sk, (struct sockaddr_in *)sap); + break; + case PF_INET6: + if (addrlen < sizeof(struct sockaddr_in6)) + return -EINVAL; + rc = smk_ipv6_port_check(sock->sk, sap, SMK_CONNECTING); + break; + } + return rc; } /** @@ -2792,22 +2984,32 @@ static int smack_unix_may_send(struct socket *sock, struct socket *other) * @msg: the message * @size: the size of the message * - * Return 0 if the current subject can write to the destination - * host. This is only a question if the destination is a single - * label host. + * Return 0 if the current subject can write to the destination host. + * For IPv4 this is only a question if the destination is a single label host. + * For IPv6 this is a check against the label of the port. */ static int smack_socket_sendmsg(struct socket *sock, struct msghdr *msg, int size) { struct sockaddr_in *sip = (struct sockaddr_in *) msg->msg_name; + struct sockaddr *sap = (struct sockaddr *) msg->msg_name; + int rc = 0; /* * Perfectly reasonable for this to be NULL */ - if (sip == NULL || sip->sin_family != AF_INET) + if (sip == NULL) return 0; - return smack_netlabel_send(sock->sk, sip); + switch (sip->sin_family) { + case AF_INET: + rc = smack_netlabel_send(sock->sk, sip); + break; + case AF_INET6: + rc = smk_ipv6_port_check(sock->sk, sap, SMK_SENDING); + break; + } + return rc; } /** @@ -2878,6 +3080,54 @@ static char *smack_from_secattr(struct netlbl_lsm_secattr *sap, return smack_net_ambient; } +static int smk_skb_to_addr_ipv6(struct sk_buff *skb, struct sockaddr *sap) +{ + struct sockaddr_in6 *sip = (struct sockaddr_in6 *)sap; + u8 nexthdr; + int offset; + int proto = -EINVAL; + struct ipv6hdr _ipv6h; + struct ipv6hdr *ip6; + __be16 frag_off; + struct tcphdr _tcph, *th; + struct udphdr _udph, *uh; + struct dccp_hdr _dccph, *dh; + + sip->sin6_port = 0; + + offset = skb_network_offset(skb); + ip6 = skb_header_pointer(skb, offset, sizeof(_ipv6h), &_ipv6h); + if (ip6 == NULL) + return -EINVAL; + sip->sin6_addr = ip6->saddr; + + nexthdr = ip6->nexthdr; + offset += sizeof(_ipv6h); + offset = ipv6_skip_exthdr(skb, offset, &nexthdr, &frag_off); + if (offset < 0) + return -EINVAL; + + proto = nexthdr; + switch (proto) { + case IPPROTO_TCP: + th = skb_header_pointer(skb, offset, sizeof(_tcph), &_tcph); + if (th != NULL) + sip->sin6_port = th->source; + break; + case IPPROTO_UDP: + uh = skb_header_pointer(skb, offset, sizeof(_udph), &_udph); + if (uh != NULL) + sip->sin6_port = uh->source; + break; + case IPPROTO_DCCP: + dh = skb_header_pointer(skb, offset, sizeof(_dccph), &_dccph); + if (dh != NULL) + sip->sin6_port = dh->dccph_sport; + break; + } + return proto; +} + /** * smack_socket_sock_rcv_skb - Smack packet delivery access check * @sk: socket @@ -2889,43 +3139,52 @@ static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb) { struct netlbl_lsm_secattr secattr; struct socket_smack *ssp = sk->sk_security; + struct sockaddr sadd; char *csp; - int rc; + int rc = 0; struct smk_audit_info ad; #ifdef CONFIG_AUDIT struct lsm_network_audit net; #endif - if (sk->sk_family != PF_INET && sk->sk_family != PF_INET6) - return 0; - - /* - * Translate what netlabel gave us. - */ - netlbl_secattr_init(&secattr); + switch (sk->sk_family) { + case PF_INET: + /* + * Translate what netlabel gave us. + */ + netlbl_secattr_init(&secattr); - rc = netlbl_skbuff_getattr(skb, sk->sk_family, &secattr); - if (rc == 0) - csp = smack_from_secattr(&secattr, ssp); - else - csp = smack_net_ambient; + rc = netlbl_skbuff_getattr(skb, sk->sk_family, &secattr); + if (rc == 0) + csp = smack_from_secattr(&secattr, ssp); + else + csp = smack_net_ambient; - netlbl_secattr_destroy(&secattr); + netlbl_secattr_destroy(&secattr); #ifdef CONFIG_AUDIT - smk_ad_init_net(&ad, __func__, LSM_AUDIT_DATA_NET, &net); - ad.a.u.net->family = sk->sk_family; - ad.a.u.net->netif = skb->skb_iif; - ipv4_skb_to_auditdata(skb, &ad.a, NULL); + smk_ad_init_net(&ad, __func__, LSM_AUDIT_DATA_NET, &net); + ad.a.u.net->family = sk->sk_family; + ad.a.u.net->netif = skb->skb_iif; + ipv4_skb_to_auditdata(skb, &ad.a, NULL); #endif - /* - * Receiving a packet requires that the other end - * be able to write here. Read access is not required. - * This is the simplist possible security model - * for networking. - */ - rc = smk_access(csp, ssp->smk_in, MAY_WRITE, &ad); - if (rc != 0) - netlbl_skbuff_err(skb, rc, 0); + /* + * Receiving a packet requires that the other end + * be able to write here. Read access is not required. + * This is the simplist possible security model + * for networking. + */ + rc = smk_access(csp, ssp->smk_in, MAY_WRITE, &ad); + if (rc != 0) + netlbl_skbuff_err(skb, rc, 0); + break; + case PF_INET6: + rc = smk_skb_to_addr_ipv6(skb, &sadd); + if (rc == IPPROTO_UDP || rc == IPPROTO_TCP) + rc = smk_ipv6_port_check(sk, &sadd, SMK_RECEIVING); + else + rc = 0; + break; + } return rc; } @@ -3063,9 +3322,17 @@ static int smack_inet_conn_request(struct sock *sk, struct sk_buff *skb, struct lsm_network_audit net; #endif - /* handle mapped IPv4 packets arriving via IPv6 sockets */ - if (family == PF_INET6 && skb->protocol == htons(ETH_P_IP)) - family = PF_INET; + if (family == PF_INET6) { + /* + * Handle mapped IPv4 packets arriving + * via IPv6 sockets. Don't set up netlabel + * processing on IPv6. + */ + if (skb->protocol == htons(ETH_P_IP)) + family = PF_INET; + else + return 0; + } netlbl_secattr_init(&secattr); rc = netlbl_skbuff_getattr(skb, family, &secattr); @@ -3498,6 +3765,7 @@ struct security_operations smack_ops = { .unix_may_send = smack_unix_may_send, .socket_post_create = smack_socket_post_create, + .socket_bind = smack_socket_bind, .socket_connect = smack_socket_connect, .socket_sendmsg = smack_socket_sendmsg, .socket_sock_rcv_skb = smack_socket_sock_rcv_skb, -- cgit v1.2.3-59-g8ed1b From 2f823ff8bec03a1e6f9e11fd0c4d54e4c7d09532 Mon Sep 17 00:00:00 2001 From: Casey Schaufler Date: Wed, 22 May 2013 18:43:03 -0700 Subject: Smack: Improve access check performance Each Smack label that the kernel has seen is added to a list of labels. The list of access rules for a given subject label hangs off of the label list entry for the label. This patch changes the structures that contain subject labels to point at the label list entry rather that the label itself. Doing so removes a label list lookup in smk_access() that was accounting for the largest single chunk of Smack overhead. Targeted for git://git.gitorious.org/smack-next/kernel.git Signed-off-by: Casey Schaufler --- security/smack/smack.h | 108 +++++++------- security/smack/smack_access.c | 41 +++--- security/smack/smack_lsm.c | 331 +++++++++++++++++++++++------------------- security/smack/smackfs.c | 51 +++---- 4 files changed, 282 insertions(+), 249 deletions(-) (limited to 'security') diff --git a/security/smack/smack.h b/security/smack/smack.h index bb28e099abfe..159f25bfcf45 100644 --- a/security/smack/smack.h +++ b/security/smack/smack.h @@ -28,6 +28,38 @@ #define SMK_LABELLEN 24 #define SMK_LONGLABEL 256 +/* + * This is the repository for labels seen so that it is + * not necessary to keep allocating tiny chuncks of memory + * and so that they can be shared. + * + * Labels are never modified in place. Anytime a label + * is imported (e.g. xattrset on a file) the list is checked + * for it and it is added if it doesn't exist. The address + * is passed out in either case. Entries are added, but + * never deleted. + * + * Since labels are hanging around anyway it doesn't + * hurt to maintain a secid for those awkward situations + * where kernel components that ought to use LSM independent + * interfaces don't. The secid should go away when all of + * these components have been repaired. + * + * The cipso value associated with the label gets stored here, too. + * + * Keep the access rules for this subject label here so that + * the entire set of rules does not need to be examined every + * time. + */ +struct smack_known { + struct list_head list; + char *smk_known; + u32 smk_secid; + struct netlbl_lsm_secattr smk_netlabel; /* on wire labels */ + struct list_head smk_rules; /* access rules */ + struct mutex smk_rules_lock; /* lock for rules */ +}; + /* * Maximum number of bytes for the levels in a CIPSO IP option. * Why 23? CIPSO is constrained to 30, so a 32 byte buffer is @@ -46,25 +78,25 @@ struct superblock_smack { }; struct socket_smack { - char *smk_out; /* outbound label */ - char *smk_in; /* inbound label */ - char *smk_packet; /* TCP peer label */ + struct smack_known *smk_out; /* outbound label */ + char *smk_in; /* inbound label */ + char *smk_packet; /* TCP peer label */ }; /* * Inode smack data */ struct inode_smack { - char *smk_inode; /* label of the fso */ - char *smk_task; /* label of the task */ - char *smk_mmap; /* label of the mmap domain */ - struct mutex smk_lock; /* initialization lock */ - int smk_flags; /* smack inode flags */ + char *smk_inode; /* label of the fso */ + struct smack_known *smk_task; /* label of the task */ + struct smack_known *smk_mmap; /* label of the mmap domain */ + struct mutex smk_lock; /* initialization lock */ + int smk_flags; /* smack inode flags */ }; struct task_smack { - char *smk_task; /* label for access control */ - char *smk_forked; /* label when forked */ + struct smack_known *smk_task; /* label for access control */ + struct smack_known *smk_forked; /* label when forked */ struct list_head smk_rules; /* per task access rules */ struct mutex smk_rules_lock; /* lock for the rules */ }; @@ -78,7 +110,7 @@ struct task_smack { */ struct smack_rule { struct list_head list; - char *smk_subject; + struct smack_known *smk_subject; char *smk_object; int smk_access; }; @@ -101,39 +133,7 @@ struct smk_port_label { struct sock *smk_sock; /* socket initialized on */ unsigned short smk_port; /* the port number */ char *smk_in; /* incoming label */ - char *smk_out; /* outgoing label */ -}; - -/* - * This is the repository for labels seen so that it is - * not necessary to keep allocating tiny chuncks of memory - * and so that they can be shared. - * - * Labels are never modified in place. Anytime a label - * is imported (e.g. xattrset on a file) the list is checked - * for it and it is added if it doesn't exist. The address - * is passed out in either case. Entries are added, but - * never deleted. - * - * Since labels are hanging around anyway it doesn't - * hurt to maintain a secid for those awkward situations - * where kernel components that ought to use LSM independent - * interfaces don't. The secid should go away when all of - * these components have been repaired. - * - * The cipso value associated with the label gets stored here, too. - * - * Keep the access rules for this subject label here so that - * the entire set of rules does not need to be examined every - * time. - */ -struct smack_known { - struct list_head list; - char *smk_known; - u32 smk_secid; - struct netlbl_lsm_secattr smk_netlabel; /* on wire labels */ - struct list_head smk_rules; /* access rules */ - struct mutex smk_rules_lock; /* lock for rules */ + struct smack_known *smk_out; /* outgoing label */ }; /* @@ -214,9 +214,9 @@ struct inode_smack *new_inode_smack(char *); * These functions are in smack_access.c */ int smk_access_entry(char *, char *, struct list_head *); -int smk_access(char *, char *, int, struct smk_audit_info *); +int smk_access(struct smack_known *, char *, int, struct smk_audit_info *); int smk_curacc(char *, u32, struct smk_audit_info *); -char *smack_from_secid(const u32); +struct smack_known *smack_from_secid(const u32); char *smk_parse_smack(const char *string, int len); int smk_netlbl_mls(int, char *, struct netlbl_lsm_secattr *, int); char *smk_import(const char *, int); @@ -229,7 +229,7 @@ u32 smack_to_secid(const char *); */ extern int smack_cipso_direct; extern int smack_cipso_mapped; -extern char *smack_net_ambient; +extern struct smack_known *smack_net_ambient; extern char *smack_onlycap; extern const char *smack_cipso_option; @@ -265,17 +265,17 @@ static inline char *smk_of_inode(const struct inode *isp) } /* - * Present a pointer to the smack label in an task blob. + * Present a pointer to the smack label entry in an task blob. */ -static inline char *smk_of_task(const struct task_smack *tsp) +static inline struct smack_known *smk_of_task(const struct task_smack *tsp) { return tsp->smk_task; } /* - * Present a pointer to the forked smack label in an task blob. + * Present a pointer to the forked smack label entry in an task blob. */ -static inline char *smk_of_forked(const struct task_smack *tsp) +static inline struct smack_known *smk_of_forked(const struct task_smack *tsp) { return tsp->smk_forked; } @@ -283,7 +283,7 @@ static inline char *smk_of_forked(const struct task_smack *tsp) /* * Present a pointer to the smack label in the current task blob. */ -static inline char *smk_of_current(void) +static inline struct smack_known *smk_of_current(void) { return smk_of_task(current_security()); } @@ -294,9 +294,11 @@ static inline char *smk_of_current(void) */ static inline int smack_privileged(int cap) { + struct smack_known *skp = smk_of_current(); + if (!capable(cap)) return 0; - if (smack_onlycap == NULL || smack_onlycap == smk_of_current()) + if (smack_onlycap == NULL || smack_onlycap == skp->smk_known) return 1; return 0; } diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c index 2e397a88d410..53f2327a592f 100644 --- a/security/smack/smack_access.c +++ b/security/smack/smack_access.c @@ -93,7 +93,7 @@ int smk_access_entry(char *subject_label, char *object_label, list_for_each_entry_rcu(srp, rule_list, list) { if (srp->smk_object == object_label && - srp->smk_subject == subject_label) { + srp->smk_subject->smk_known == subject_label) { may = srp->smk_access; break; } @@ -104,7 +104,7 @@ int smk_access_entry(char *subject_label, char *object_label, /** * smk_access - determine if a subject has a specific access to an object - * @subject_label: a pointer to the subject's Smack label + * @subject_known: a pointer to the subject's Smack label entry * @object_label: a pointer to the object's Smack label * @request: the access requested, in "MAY" format * @a : a pointer to the audit data @@ -115,10 +115,9 @@ int smk_access_entry(char *subject_label, char *object_label, * * Smack labels are shared on smack_list */ -int smk_access(char *subject_label, char *object_label, int request, - struct smk_audit_info *a) +int smk_access(struct smack_known *subject_known, char *object_label, + int request, struct smk_audit_info *a) { - struct smack_known *skp; int may = MAY_NOT; int rc = 0; @@ -127,7 +126,7 @@ int smk_access(char *subject_label, char *object_label, int request, * * A star subject can't access any object. */ - if (subject_label == smack_known_star.smk_known) { + if (subject_known == &smack_known_star) { rc = -EACCES; goto out_audit; } @@ -137,7 +136,7 @@ int smk_access(char *subject_label, char *object_label, int request, * An internet subject can access any object. */ if (object_label == smack_known_web.smk_known || - subject_label == smack_known_web.smk_known) + subject_known == &smack_known_web) goto out_audit; /* * A star object can be accessed by any subject. @@ -148,7 +147,7 @@ int smk_access(char *subject_label, char *object_label, int request, * An object can be accessed in any way by a subject * with the same label. */ - if (subject_label == object_label) + if (subject_known->smk_known == object_label) goto out_audit; /* * A hat subject can read any object. @@ -157,7 +156,7 @@ int smk_access(char *subject_label, char *object_label, int request, if ((request & MAY_ANYREAD) == request) { if (object_label == smack_known_floor.smk_known) goto out_audit; - if (subject_label == smack_known_hat.smk_known) + if (subject_known == &smack_known_hat) goto out_audit; } /* @@ -167,9 +166,9 @@ int smk_access(char *subject_label, char *object_label, int request, * good. A negative response from smk_access_entry() * indicates there is no entry for this pair. */ - skp = smk_find_entry(subject_label); rcu_read_lock(); - may = smk_access_entry(subject_label, object_label, &skp->smk_rules); + may = smk_access_entry(subject_known->smk_known, object_label, + &subject_known->smk_rules); rcu_read_unlock(); if (may > 0 && (request & may) == request) @@ -179,7 +178,8 @@ int smk_access(char *subject_label, char *object_label, int request, out_audit: #ifdef CONFIG_AUDIT if (a) - smack_log(subject_label, object_label, request, rc, a); + smack_log(subject_known->smk_known, object_label, request, + rc, a); #endif return rc; } @@ -198,20 +198,21 @@ out_audit: int smk_curacc(char *obj_label, u32 mode, struct smk_audit_info *a) { struct task_smack *tsp = current_security(); - char *sp = smk_of_task(tsp); + struct smack_known *skp = smk_of_task(tsp); int may; int rc; /* * Check the global rule list */ - rc = smk_access(sp, obj_label, mode, NULL); + rc = smk_access(skp, obj_label, mode, NULL); if (rc == 0) { /* * If there is an entry in the task's rule list * it can further restrict access. */ - may = smk_access_entry(sp, obj_label, &tsp->smk_rules); + may = smk_access_entry(skp->smk_known, obj_label, + &tsp->smk_rules); if (may < 0) goto out_audit; if ((mode & may) == mode) @@ -228,7 +229,7 @@ int smk_curacc(char *obj_label, u32 mode, struct smk_audit_info *a) out_audit: #ifdef CONFIG_AUDIT if (a) - smack_log(sp, obj_label, mode, rc, a); + smack_log(skp->smk_known, obj_label, mode, rc, a); #endif return rc; } @@ -513,10 +514,10 @@ char *smk_import(const char *string, int len) * smack_from_secid - find the Smack label associated with a secid * @secid: an integer that might be associated with a Smack label * - * Returns a pointer to the appropriate Smack label if there is one, + * Returns a pointer to the appropriate Smack label entry if there is one, * otherwise a pointer to the invalid Smack label. */ -char *smack_from_secid(const u32 secid) +struct smack_known *smack_from_secid(const u32 secid) { struct smack_known *skp; @@ -524,7 +525,7 @@ char *smack_from_secid(const u32 secid) list_for_each_entry_rcu(skp, &smack_known_list, list) { if (skp->smk_secid == secid) { rcu_read_unlock(); - return skp->smk_known; + return skp; } } @@ -533,7 +534,7 @@ char *smack_from_secid(const u32 secid) * of a secid that is not on the list. */ rcu_read_unlock(); - return smack_known_invalid.smk_known; + return &smack_known_invalid; } /** diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 609e89de3c24..3669d9f9824e 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -62,11 +62,12 @@ LIST_HEAD(smk_ipv6_port_list); * Returns a pointer to the master list entry for the Smack label * or NULL if there was no label to fetch. */ -static char *smk_fetch(const char *name, struct inode *ip, struct dentry *dp) +static struct smack_known *smk_fetch(const char *name, struct inode *ip, + struct dentry *dp) { int rc; char *buffer; - char *result = NULL; + struct smack_known *skp = NULL; if (ip->i_op->getxattr == NULL) return NULL; @@ -77,11 +78,11 @@ static char *smk_fetch(const char *name, struct inode *ip, struct dentry *dp) rc = ip->i_op->getxattr(dp, name, buffer, SMK_LONGLABEL); if (rc > 0) - result = smk_import(buffer, rc); + skp = smk_import_entry(buffer, rc); kfree(buffer); - return result; + return skp; } /** @@ -111,7 +112,8 @@ struct inode_smack *new_inode_smack(char *smack) * * Returns the new blob or NULL if there's no memory available */ -static struct task_smack *new_task_smack(char *task, char *forked, gfp_t gfp) +static struct task_smack *new_task_smack(struct smack_known *task, + struct smack_known *forked, gfp_t gfp) { struct task_smack *tsp; @@ -173,17 +175,17 @@ static int smack_ptrace_access_check(struct task_struct *ctp, unsigned int mode) { int rc; struct smk_audit_info ad; - char *tsp; + struct smack_known *skp; rc = cap_ptrace_access_check(ctp, mode); if (rc != 0) return rc; - tsp = smk_of_task(task_security(ctp)); + skp = smk_of_task(task_security(ctp)); smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_TASK); smk_ad_setfield_u_tsk(&ad, ctp); - rc = smk_curacc(tsp, MAY_READWRITE, &ad); + rc = smk_curacc(skp->smk_known, MAY_READWRITE, &ad); return rc; } @@ -199,17 +201,17 @@ static int smack_ptrace_traceme(struct task_struct *ptp) { int rc; struct smk_audit_info ad; - char *tsp; + struct smack_known *skp; rc = cap_ptrace_traceme(ptp); if (rc != 0) return rc; - tsp = smk_of_task(task_security(ptp)); + skp = smk_of_task(task_security(ptp)); smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_TASK); smk_ad_setfield_u_tsk(&ad, ptp); - rc = smk_curacc(tsp, MAY_READWRITE, &ad); + rc = smk_curacc(skp->smk_known, MAY_READWRITE, &ad); return rc; } @@ -224,12 +226,12 @@ static int smack_ptrace_traceme(struct task_struct *ptp) static int smack_syslog(int typefrom_file) { int rc = 0; - char *sp = smk_of_current(); + struct smack_known *skp = smk_of_current(); if (smack_privileged(CAP_MAC_OVERRIDE)) return 0; - if (sp != smack_known_floor.smk_known) + if (skp != &smack_known_floor) rc = -EACCES; return rc; @@ -533,7 +535,9 @@ static int smack_bprm_secureexec(struct linux_binprm *bprm) */ static int smack_inode_alloc_security(struct inode *inode) { - inode->i_security = new_inode_smack(smk_of_current()); + struct smack_known *skp = smk_of_current(); + + inode->i_security = new_inode_smack(skp->smk_known); if (inode->i_security == NULL) return -ENOMEM; return 0; @@ -566,9 +570,8 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir, const struct qstr *qstr, char **name, void **value, size_t *len) { - struct smack_known *skp; struct inode_smack *issp = inode->i_security; - char *csp = smk_of_current(); + struct smack_known *skp = smk_of_current(); char *isp = smk_of_inode(inode); char *dsp = smk_of_inode(dir); int may; @@ -580,9 +583,8 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir, } if (value) { - skp = smk_find_entry(csp); rcu_read_lock(); - may = smk_access_entry(csp, dsp, &skp->smk_rules); + may = smk_access_entry(skp->smk_known, dsp, &skp->smk_rules); rcu_read_unlock(); /* @@ -871,29 +873,31 @@ static int smack_inode_setxattr(struct dentry *dentry, const char *name, static void smack_inode_post_setxattr(struct dentry *dentry, const char *name, const void *value, size_t size, int flags) { - char *nsp; + struct smack_known *skp; struct inode_smack *isp = dentry->d_inode->i_security; + if (strcmp(name, XATTR_NAME_SMACKTRANSMUTE) == 0) { + isp->smk_flags |= SMK_INODE_TRANSMUTE; + return; + } + + skp = smk_import_entry(value, size); if (strcmp(name, XATTR_NAME_SMACK) == 0) { - nsp = smk_import(value, size); - if (nsp != NULL) - isp->smk_inode = nsp; + if (skp != NULL) + isp->smk_inode = skp->smk_known; else isp->smk_inode = smack_known_invalid.smk_known; } else if (strcmp(name, XATTR_NAME_SMACKEXEC) == 0) { - nsp = smk_import(value, size); - if (nsp != NULL) - isp->smk_task = nsp; + if (skp != NULL) + isp->smk_task = skp; else - isp->smk_task = smack_known_invalid.smk_known; + isp->smk_task = &smack_known_invalid; } else if (strcmp(name, XATTR_NAME_SMACKMMAP) == 0) { - nsp = smk_import(value, size); - if (nsp != NULL) - isp->smk_mmap = nsp; + if (skp != NULL) + isp->smk_mmap = skp; else - isp->smk_mmap = smack_known_invalid.smk_known; - } else if (strcmp(name, XATTR_NAME_SMACKTRANSMUTE) == 0) - isp->smk_flags |= SMK_INODE_TRANSMUTE; + isp->smk_mmap = &smack_known_invalid; + } return; } @@ -999,7 +1003,7 @@ static int smack_inode_getsecurity(const struct inode *inode, if (strcmp(name, XATTR_SMACK_IPIN) == 0) isp = ssp->smk_in; else if (strcmp(name, XATTR_SMACK_IPOUT) == 0) - isp = ssp->smk_out; + isp = ssp->smk_out->smk_known; else return -EOPNOTSUPP; @@ -1079,7 +1083,9 @@ static int smack_file_permission(struct file *file, int mask) */ static int smack_file_alloc_security(struct file *file) { - file->f_security = smk_of_current(); + struct smack_known *skp = smk_of_current(); + + file->f_security = skp->smk_known; return 0; } @@ -1190,10 +1196,9 @@ static int smack_mmap_file(struct file *file, unsigned long flags) { struct smack_known *skp; + struct smack_known *mkp; struct smack_rule *srp; struct task_smack *tsp; - char *sp; - char *msmack; char *osmack; struct inode_smack *isp; int may; @@ -1207,11 +1212,10 @@ static int smack_mmap_file(struct file *file, isp = file_inode(file)->i_security; if (isp->smk_mmap == NULL) return 0; - msmack = isp->smk_mmap; + mkp = isp->smk_mmap; tsp = current_security(); - sp = smk_of_current(); - skp = smk_find_entry(sp); + skp = smk_of_current(); rc = 0; rcu_read_lock(); @@ -1225,13 +1229,13 @@ static int smack_mmap_file(struct file *file, /* * Matching labels always allows access. */ - if (msmack == osmack) + if (mkp->smk_known == osmack) continue; /* * If there is a matching local rule take * that into account as well. */ - may = smk_access_entry(srp->smk_subject, osmack, + may = smk_access_entry(srp->smk_subject->smk_known, osmack, &tsp->smk_rules); if (may == -ENOENT) may = srp->smk_access; @@ -1249,8 +1253,8 @@ static int smack_mmap_file(struct file *file, * If there isn't one a SMACK64MMAP subject * can't have as much access as current. */ - skp = smk_find_entry(msmack); - mmay = smk_access_entry(msmack, osmack, &skp->smk_rules); + mmay = smk_access_entry(mkp->smk_known, osmack, + &mkp->smk_rules); if (mmay == -ENOENT) { rc = -EACCES; break; @@ -1259,7 +1263,8 @@ static int smack_mmap_file(struct file *file, * If there is a local entry it modifies the * potential access, too. */ - tmay = smk_access_entry(msmack, osmack, &tsp->smk_rules); + tmay = smk_access_entry(mkp->smk_known, osmack, + &tsp->smk_rules); if (tmay != -ENOENT) mmay &= tmay; @@ -1288,7 +1293,9 @@ static int smack_mmap_file(struct file *file, */ static int smack_file_set_fowner(struct file *file) { - file->f_security = smk_of_current(); + struct smack_known *skp = smk_of_current(); + + file->f_security = skp->smk_known; return 0; } @@ -1306,9 +1313,10 @@ static int smack_file_set_fowner(struct file *file) static int smack_file_send_sigiotask(struct task_struct *tsk, struct fown_struct *fown, int signum) { + struct smack_known *skp; + struct smack_known *tkp = smk_of_task(tsk->cred->security); struct file *file; int rc; - char *tsp = smk_of_task(tsk->cred->security); struct smk_audit_info ad; /* @@ -1317,13 +1325,14 @@ static int smack_file_send_sigiotask(struct task_struct *tsk, file = container_of(fown, struct file, f_owner); /* we don't log here as rc can be overriden */ - rc = smk_access(file->f_security, tsp, MAY_WRITE, NULL); + skp = smk_find_entry(file->f_security); + rc = smk_access(skp, tkp->smk_known, MAY_WRITE, NULL); if (rc != 0 && has_capability(tsk, CAP_MAC_OVERRIDE)) rc = 0; smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_TASK); smk_ad_setfield_u_tsk(&ad, tsk); - smack_log(file->f_security, tsp, MAY_WRITE, rc, &ad); + smack_log(file->f_security, tkp->smk_known, MAY_WRITE, rc, &ad); return rc; } @@ -1478,12 +1487,12 @@ static void smack_cred_transfer(struct cred *new, const struct cred *old) static int smack_kernel_act_as(struct cred *new, u32 secid) { struct task_smack *new_tsp = new->security; - char *smack = smack_from_secid(secid); + struct smack_known *skp = smack_from_secid(secid); - if (smack == NULL) + if (skp == NULL) return -EINVAL; - new_tsp->smk_task = smack; + new_tsp->smk_task = skp; return 0; } @@ -1501,8 +1510,8 @@ static int smack_kernel_create_files_as(struct cred *new, struct inode_smack *isp = inode->i_security; struct task_smack *tsp = new->security; - tsp->smk_forked = isp->smk_inode; - tsp->smk_task = isp->smk_inode; + tsp->smk_forked = smk_find_entry(isp->smk_inode); + tsp->smk_task = tsp->smk_forked; return 0; } @@ -1518,10 +1527,11 @@ static int smk_curacc_on_task(struct task_struct *p, int access, const char *caller) { struct smk_audit_info ad; + struct smack_known *skp = smk_of_task(task_security(p)); smk_ad_init(&ad, caller, LSM_AUDIT_DATA_TASK); smk_ad_setfield_u_tsk(&ad, p); - return smk_curacc(smk_of_task(task_security(p)), access, &ad); + return smk_curacc(skp->smk_known, access, &ad); } /** @@ -1567,7 +1577,9 @@ static int smack_task_getsid(struct task_struct *p) */ static void smack_task_getsecid(struct task_struct *p, u32 *secid) { - *secid = smack_to_secid(smk_of_task(task_security(p))); + struct smack_known *skp = smk_of_task(task_security(p)); + + *secid = skp->smk_secid; } /** @@ -1671,6 +1683,8 @@ static int smack_task_kill(struct task_struct *p, struct siginfo *info, int sig, u32 secid) { struct smk_audit_info ad; + struct smack_known *skp; + struct smack_known *tkp = smk_of_task(task_security(p)); smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_TASK); smk_ad_setfield_u_tsk(&ad, p); @@ -1679,15 +1693,14 @@ static int smack_task_kill(struct task_struct *p, struct siginfo *info, * can write the receiver. */ if (secid == 0) - return smk_curacc(smk_of_task(task_security(p)), MAY_WRITE, - &ad); + return smk_curacc(tkp->smk_known, MAY_WRITE, &ad); /* * If the secid isn't 0 we're dealing with some USB IO * specific behavior. This is not clean. For one thing * we can't take privilege into account. */ - return smk_access(smack_from_secid(secid), - smk_of_task(task_security(p)), MAY_WRITE, &ad); + skp = smack_from_secid(secid); + return smk_access(skp, tkp->smk_known, MAY_WRITE, &ad); } /** @@ -1719,7 +1732,9 @@ static int smack_task_wait(struct task_struct *p) static void smack_task_to_inode(struct task_struct *p, struct inode *inode) { struct inode_smack *isp = inode->i_security; - isp->smk_inode = smk_of_task(task_security(p)); + struct smack_known *skp = smk_of_task(task_security(p)); + + isp->smk_inode = skp->smk_known; } /* @@ -1738,15 +1753,15 @@ static void smack_task_to_inode(struct task_struct *p, struct inode *inode) */ static int smack_sk_alloc_security(struct sock *sk, int family, gfp_t gfp_flags) { - char *csp = smk_of_current(); + struct smack_known *skp = smk_of_current(); struct socket_smack *ssp; ssp = kzalloc(sizeof(struct socket_smack), gfp_flags); if (ssp == NULL) return -ENOMEM; - ssp->smk_in = csp; - ssp->smk_out = csp; + ssp->smk_in = skp->smk_known; + ssp->smk_out = skp; ssp->smk_packet = NULL; sk->sk_security = ssp; @@ -1833,7 +1848,7 @@ static int smack_netlabel(struct sock *sk, int labeled) labeled == SMACK_UNLABELED_SOCKET) netlbl_sock_delattr(sk); else { - skp = smk_find_entry(ssp->smk_out); + skp = ssp->smk_out; rc = netlbl_sock_setattr(sk, sk->sk_family, &skp->smk_netlabel); } @@ -1856,6 +1871,7 @@ static int smack_netlabel(struct sock *sk, int labeled) */ static int smack_netlabel_send(struct sock *sk, struct sockaddr_in *sap) { + struct smack_known *skp; int rc; int sk_lbl; char *hostsp; @@ -1874,7 +1890,8 @@ static int smack_netlabel_send(struct sock *sk, struct sockaddr_in *sap) ad.a.u.net->v4info.daddr = sap->sin_addr.s_addr; #endif sk_lbl = SMACK_UNLABELED_SOCKET; - rc = smk_access(ssp->smk_out, hostsp, MAY_WRITE, &ad); + skp = ssp->smk_out; + rc = smk_access(skp, hostsp, MAY_WRITE, &ad); } else { sk_lbl = SMACK_CIPSO_SOCKET; rc = 0; @@ -1974,8 +1991,8 @@ static int smk_ipv6_port_check(struct sock *sk, struct sockaddr *address, struct sockaddr_in6 *addr6; struct smk_port_label *spp; struct socket_smack *ssp = sk->sk_security; + struct smack_known *skp; unsigned short port = 0; - char *subject; char *object; struct smk_audit_info ad; #ifdef CONFIG_AUDIT @@ -1983,11 +2000,11 @@ static int smk_ipv6_port_check(struct sock *sk, struct sockaddr *address, #endif if (act == SMK_RECEIVING) { - subject = smack_net_ambient; + skp = smack_net_ambient; object = ssp->smk_in; } else { - subject = ssp->smk_out; - object = smack_net_ambient; + skp = ssp->smk_out; + object = smack_net_ambient->smk_known; } /* @@ -2008,7 +2025,7 @@ static int smk_ipv6_port_check(struct sock *sk, struct sockaddr *address, * It's local so the send check has to have passed. */ if (act == SMK_RECEIVING) { - subject = smack_known_web.smk_known; + skp = &smack_known_web; goto auditout; } @@ -2017,7 +2034,7 @@ static int smk_ipv6_port_check(struct sock *sk, struct sockaddr *address, continue; object = spp->smk_in; if (act == SMK_CONNECTING) - ssp->smk_packet = spp->smk_out; + ssp->smk_packet = spp->smk_out->smk_known; break; } @@ -2032,7 +2049,7 @@ auditout: else ad.a.u.net->v6info.daddr = addr6->sin6_addr; #endif - return smk_access(subject, object, MAY_WRITE, &ad); + return smk_access(skp, object, MAY_WRITE, &ad); } /** @@ -2050,7 +2067,7 @@ auditout: static int smack_inode_setsecurity(struct inode *inode, const char *name, const void *value, size_t size, int flags) { - char *sp; + struct smack_known *skp; struct inode_smack *nsp = inode->i_security; struct socket_smack *ssp; struct socket *sock; @@ -2059,12 +2076,12 @@ static int smack_inode_setsecurity(struct inode *inode, const char *name, if (value == NULL || size > SMK_LONGLABEL || size == 0) return -EACCES; - sp = smk_import(value, size); - if (sp == NULL) + skp = smk_import_entry(value, size); + if (skp == NULL) return -EINVAL; if (strcmp(name, XATTR_SMACK_SUFFIX) == 0) { - nsp->smk_inode = sp; + nsp->smk_inode = skp->smk_known; nsp->smk_flags |= SMK_INODE_INSTANT; return 0; } @@ -2081,9 +2098,9 @@ static int smack_inode_setsecurity(struct inode *inode, const char *name, ssp = sock->sk->sk_security; if (strcmp(name, XATTR_SMACK_IPIN) == 0) - ssp->smk_in = sp; + ssp->smk_in = skp->smk_known; else if (strcmp(name, XATTR_SMACK_IPOUT) == 0) { - ssp->smk_out = sp; + ssp->smk_out = skp; if (sock->sk->sk_family == PF_INET) { rc = smack_netlabel(sock->sk, SMACK_CIPSO_SOCKET); if (rc != 0) @@ -2203,7 +2220,9 @@ static int smack_flags_to_may(int flags) */ static int smack_msg_msg_alloc_security(struct msg_msg *msg) { - msg->security = smk_of_current(); + struct smack_known *skp = smk_of_current(); + + msg->security = skp->smk_known; return 0; } @@ -2238,8 +2257,9 @@ static char *smack_of_shm(struct shmid_kernel *shp) static int smack_shm_alloc_security(struct shmid_kernel *shp) { struct kern_ipc_perm *isp = &shp->shm_perm; + struct smack_known *skp = smk_of_current(); - isp->security = smk_of_current(); + isp->security = skp->smk_known; return 0; } @@ -2361,8 +2381,9 @@ static char *smack_of_sem(struct sem_array *sma) static int smack_sem_alloc_security(struct sem_array *sma) { struct kern_ipc_perm *isp = &sma->sem_perm; + struct smack_known *skp = smk_of_current(); - isp->security = smk_of_current(); + isp->security = skp->smk_known; return 0; } @@ -2479,8 +2500,9 @@ static int smack_sem_semop(struct sem_array *sma, struct sembuf *sops, static int smack_msg_queue_alloc_security(struct msg_queue *msq) { struct kern_ipc_perm *kisp = &msq->q_perm; + struct smack_known *skp = smk_of_current(); - kisp->security = smk_of_current(); + kisp->security = skp->smk_known; return 0; } @@ -2652,8 +2674,8 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode) struct super_block *sbp; struct superblock_smack *sbsp; struct inode_smack *isp; - char *csp = smk_of_current(); - char *fetched; + struct smack_known *skp; + struct smack_known *ckp = smk_of_current(); char *final; char trattr[TRANS_TRUE_SIZE]; int transflag = 0; @@ -2720,7 +2742,7 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode) * Programs that change smack have to treat the * pty with respect. */ - final = csp; + final = ckp->smk_known; break; case SOCKFS_MAGIC: /* @@ -2775,9 +2797,9 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode) * Get the dentry for xattr. */ dp = dget(opt_dentry); - fetched = smk_fetch(XATTR_NAME_SMACK, inode, dp); - if (fetched != NULL) - final = fetched; + skp = smk_fetch(XATTR_NAME_SMACK, inode, dp); + if (skp != NULL) + final = skp->smk_known; /* * Transmuting directory @@ -2817,7 +2839,7 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode) } if (final == NULL) - isp->smk_inode = csp; + isp->smk_inode = ckp->smk_known; else isp->smk_inode = final; @@ -2840,13 +2862,14 @@ unlockandout: */ static int smack_getprocattr(struct task_struct *p, char *name, char **value) { + struct smack_known *skp = smk_of_task(task_security(p)); char *cp; int slen; if (strcmp(name, "current") != 0) return -EINVAL; - cp = kstrdup(smk_of_task(task_security(p)), GFP_KERNEL); + cp = kstrdup(skp->smk_known, GFP_KERNEL); if (cp == NULL) return -ENOMEM; @@ -2872,7 +2895,7 @@ static int smack_setprocattr(struct task_struct *p, char *name, { struct task_smack *tsp; struct cred *new; - char *newsmack; + struct smack_known *skp; /* * Changing another process' Smack value is too dangerous @@ -2890,14 +2913,14 @@ static int smack_setprocattr(struct task_struct *p, char *name, if (strcmp(name, "current") != 0) return -EINVAL; - newsmack = smk_import(value, size); - if (newsmack == NULL) + skp = smk_import_entry(value, size); + if (skp == NULL) return -EINVAL; /* * No process is ever allowed the web ("@") label. */ - if (newsmack == smack_known_web.smk_known) + if (skp == &smack_known_web) return -EPERM; new = prepare_creds(); @@ -2905,7 +2928,7 @@ static int smack_setprocattr(struct task_struct *p, char *name, return -ENOMEM; tsp = new->security; - tsp->smk_task = newsmack; + tsp->smk_task = skp; commit_creds(new); return size; @@ -2923,6 +2946,7 @@ static int smack_setprocattr(struct task_struct *p, char *name, static int smack_unix_stream_connect(struct sock *sock, struct sock *other, struct sock *newsk) { + struct smack_known *skp; struct socket_smack *ssp = sock->sk_security; struct socket_smack *osp = other->sk_security; struct socket_smack *nsp = newsk->sk_security; @@ -2936,15 +2960,17 @@ static int smack_unix_stream_connect(struct sock *sock, smk_ad_setfield_u_net_sk(&ad, other); #endif - if (!smack_privileged(CAP_MAC_OVERRIDE)) - rc = smk_access(ssp->smk_out, osp->smk_in, MAY_WRITE, &ad); + if (!smack_privileged(CAP_MAC_OVERRIDE)) { + skp = ssp->smk_out; + rc = smk_access(skp, osp->smk_in, MAY_WRITE, &ad); + } /* * Cross reference the peer labels for SO_PEERSEC. */ if (rc == 0) { - nsp->smk_packet = ssp->smk_out; - ssp->smk_packet = osp->smk_out; + nsp->smk_packet = ssp->smk_out->smk_known; + ssp->smk_packet = osp->smk_out->smk_known; } return rc; @@ -2962,8 +2988,8 @@ static int smack_unix_may_send(struct socket *sock, struct socket *other) { struct socket_smack *ssp = sock->sk->sk_security; struct socket_smack *osp = other->sk->sk_security; + struct smack_known *skp; struct smk_audit_info ad; - int rc = 0; #ifdef CONFIG_AUDIT struct lsm_network_audit net; @@ -2972,10 +2998,11 @@ static int smack_unix_may_send(struct socket *sock, struct socket *other) smk_ad_setfield_u_net_sk(&ad, other->sk); #endif - if (!smack_privileged(CAP_MAC_OVERRIDE)) - rc = smk_access(ssp->smk_out, osp->smk_in, MAY_WRITE, &ad); + if (smack_privileged(CAP_MAC_OVERRIDE)) + return 0; - return rc; + skp = ssp->smk_out; + return smk_access(skp, osp->smk_in, MAY_WRITE, &ad); } /** @@ -3017,13 +3044,12 @@ static int smack_socket_sendmsg(struct socket *sock, struct msghdr *msg, * @sap: netlabel secattr * @ssp: socket security information * - * Returns a pointer to a Smack label found on the label list. + * Returns a pointer to a Smack label entry found on the label list. */ -static char *smack_from_secattr(struct netlbl_lsm_secattr *sap, - struct socket_smack *ssp) +static struct smack_known *smack_from_secattr(struct netlbl_lsm_secattr *sap, + struct socket_smack *ssp) { - struct smack_known *kp; - char *sp; + struct smack_known *skp; int found = 0; if ((sap->flags & NETLBL_SECATTR_MLS_LVL) != 0) { @@ -3038,11 +3064,11 @@ static char *smack_from_secattr(struct netlbl_lsm_secattr *sap, * ambient value. */ rcu_read_lock(); - list_for_each_entry(kp, &smack_known_list, list) { - if (sap->attr.mls.lvl != kp->smk_netlabel.attr.mls.lvl) + list_for_each_entry(skp, &smack_known_list, list) { + if (sap->attr.mls.lvl != skp->smk_netlabel.attr.mls.lvl) continue; if (memcmp(sap->attr.mls.cat, - kp->smk_netlabel.attr.mls.cat, + skp->smk_netlabel.attr.mls.cat, SMK_CIPSOLEN) != 0) continue; found = 1; @@ -3051,17 +3077,17 @@ static char *smack_from_secattr(struct netlbl_lsm_secattr *sap, rcu_read_unlock(); if (found) - return kp->smk_known; + return skp; if (ssp != NULL && ssp->smk_in == smack_known_star.smk_known) - return smack_known_web.smk_known; - return smack_known_star.smk_known; + return &smack_known_web; + return &smack_known_star; } if ((sap->flags & NETLBL_SECATTR_SECID) != 0) { /* * Looks like a fallback, which gives us a secid. */ - sp = smack_from_secid(sap->attr.secid); + skp = smack_from_secid(sap->attr.secid); /* * This has got to be a bug because it is * impossible to specify a fallback without @@ -3069,8 +3095,8 @@ static char *smack_from_secattr(struct netlbl_lsm_secattr *sap, * it has a secid, and the only way to get a * secid is from a fallback. */ - BUG_ON(sp == NULL); - return sp; + BUG_ON(skp == NULL); + return skp; } /* * Without guidance regarding the smack value @@ -3139,8 +3165,8 @@ static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb) { struct netlbl_lsm_secattr secattr; struct socket_smack *ssp = sk->sk_security; + struct smack_known *skp; struct sockaddr sadd; - char *csp; int rc = 0; struct smk_audit_info ad; #ifdef CONFIG_AUDIT @@ -3155,9 +3181,9 @@ static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb) rc = netlbl_skbuff_getattr(skb, sk->sk_family, &secattr); if (rc == 0) - csp = smack_from_secattr(&secattr, ssp); + skp = smack_from_secattr(&secattr, ssp); else - csp = smack_net_ambient; + skp = smack_net_ambient; netlbl_secattr_destroy(&secattr); @@ -3173,7 +3199,7 @@ static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb) * This is the simplist possible security model * for networking. */ - rc = smk_access(csp, ssp->smk_in, MAY_WRITE, &ad); + rc = smk_access(skp, ssp->smk_in, MAY_WRITE, &ad); if (rc != 0) netlbl_skbuff_err(skb, rc, 0); break; @@ -3238,7 +3264,7 @@ static int smack_socket_getpeersec_dgram(struct socket *sock, { struct netlbl_lsm_secattr secattr; struct socket_smack *ssp = NULL; - char *sp; + struct smack_known *skp; int family = PF_UNSPEC; u32 s = 0; /* 0 is the invalid secid */ int rc; @@ -3254,7 +3280,7 @@ static int smack_socket_getpeersec_dgram(struct socket *sock, if (family == PF_UNIX) { ssp = sock->sk->sk_security; - s = smack_to_secid(ssp->smk_out); + s = ssp->smk_out->smk_secid; } else if (family == PF_INET || family == PF_INET6) { /* * Translate what netlabel gave us. @@ -3264,8 +3290,8 @@ static int smack_socket_getpeersec_dgram(struct socket *sock, netlbl_secattr_init(&secattr); rc = netlbl_skbuff_getattr(skb, family, &secattr); if (rc == 0) { - sp = smack_from_secattr(&secattr, ssp); - s = smack_to_secid(sp); + skp = smack_from_secattr(&secattr, ssp); + s = skp->smk_secid; } netlbl_secattr_destroy(&secattr); } @@ -3286,13 +3312,15 @@ static int smack_socket_getpeersec_dgram(struct socket *sock, static void smack_sock_graft(struct sock *sk, struct socket *parent) { struct socket_smack *ssp; + struct smack_known *skp = smk_of_current(); if (sk == NULL || (sk->sk_family != PF_INET && sk->sk_family != PF_INET6)) return; ssp = sk->sk_security; - ssp->smk_in = ssp->smk_out = smk_of_current(); + ssp->smk_in = skp->smk_known; + ssp->smk_out = skp; /* cssp->smk_packet is already set in smack_inet_csk_clone() */ } @@ -3314,7 +3342,6 @@ static int smack_inet_conn_request(struct sock *sk, struct sk_buff *skb, struct netlbl_lsm_secattr secattr; struct sockaddr_in addr; struct iphdr *hdr; - char *sp; char *hsp; int rc; struct smk_audit_info ad; @@ -3337,9 +3364,9 @@ static int smack_inet_conn_request(struct sock *sk, struct sk_buff *skb, netlbl_secattr_init(&secattr); rc = netlbl_skbuff_getattr(skb, family, &secattr); if (rc == 0) - sp = smack_from_secattr(&secattr, ssp); + skp = smack_from_secattr(&secattr, ssp); else - sp = smack_known_huh.smk_known; + skp = &smack_known_huh; netlbl_secattr_destroy(&secattr); #ifdef CONFIG_AUDIT @@ -3352,7 +3379,7 @@ static int smack_inet_conn_request(struct sock *sk, struct sk_buff *skb, * Receiving a packet requires that the other end be able to write * here. Read access is not required. */ - rc = smk_access(sp, ssp->smk_in, MAY_WRITE, &ad); + rc = smk_access(skp, ssp->smk_in, MAY_WRITE, &ad); if (rc != 0) return rc; @@ -3360,7 +3387,7 @@ static int smack_inet_conn_request(struct sock *sk, struct sk_buff *skb, * Save the peer's label in the request_sock so we can later setup * smk_packet in the child socket so that SO_PEERCRED can report it. */ - req->peer_secid = smack_to_secid(sp); + req->peer_secid = skp->smk_secid; /* * We need to decide if we want to label the incoming connection here @@ -3373,10 +3400,9 @@ static int smack_inet_conn_request(struct sock *sk, struct sk_buff *skb, hsp = smack_host_label(&addr); rcu_read_unlock(); - if (hsp == NULL) { - skp = smk_find_entry(sp); + if (hsp == NULL) rc = netlbl_req_setattr(req, &skp->smk_netlabel); - } else + else netlbl_req_delattr(req); return rc; @@ -3393,10 +3419,12 @@ static void smack_inet_csk_clone(struct sock *sk, const struct request_sock *req) { struct socket_smack *ssp = sk->sk_security; + struct smack_known *skp; - if (req->peer_secid != 0) - ssp->smk_packet = smack_from_secid(req->peer_secid); - else + if (req->peer_secid != 0) { + skp = smack_from_secid(req->peer_secid); + ssp->smk_packet = skp->smk_known; + } else ssp->smk_packet = NULL; } @@ -3422,7 +3450,9 @@ static void smack_inet_csk_clone(struct sock *sk, static int smack_key_alloc(struct key *key, const struct cred *cred, unsigned long flags) { - key->security = smk_of_task(cred->security); + struct smack_known *skp = smk_of_task(cred->security); + + key->security = skp->smk_known; return 0; } @@ -3451,7 +3481,7 @@ static int smack_key_permission(key_ref_t key_ref, { struct key *keyp; struct smk_audit_info ad; - char *tsp = smk_of_task(cred->security); + struct smack_known *tkp = smk_of_task(cred->security); keyp = key_ref_to_ptr(key_ref); if (keyp == NULL) @@ -3465,15 +3495,14 @@ static int smack_key_permission(key_ref_t key_ref, /* * This should not occur */ - if (tsp == NULL) + if (tkp == NULL) return -EACCES; #ifdef CONFIG_AUDIT smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_KEY); ad.a.u.key_struct.key = keyp->serial; ad.a.u.key_struct.key_desc = keyp->description; #endif - return smk_access(tsp, keyp->security, - MAY_READWRITE, &ad); + return smk_access(tkp, keyp->security, MAY_READWRITE, &ad); } #endif /* CONFIG_KEYS */ @@ -3555,7 +3584,7 @@ static int smack_audit_rule_known(struct audit_krule *krule) static int smack_audit_rule_match(u32 secid, u32 field, u32 op, void *vrule, struct audit_context *actx) { - char *smack; + struct smack_known *skp; char *rule = vrule; if (!rule) { @@ -3567,7 +3596,7 @@ static int smack_audit_rule_match(u32 secid, u32 field, u32 op, void *vrule, if (field != AUDIT_SUBJ_USER && field != AUDIT_OBJ_USER) return 0; - smack = smack_from_secid(secid); + skp = smack_from_secid(secid); /* * No need to do string comparisons. If a match occurs, @@ -3575,9 +3604,9 @@ static int smack_audit_rule_match(u32 secid, u32 field, u32 op, void *vrule, * label. */ if (op == Audit_equal) - return (rule == smack); + return (rule == skp->smk_known); if (op == Audit_not_equal) - return (rule != smack); + return (rule != skp->smk_known); return 0; } @@ -3605,11 +3634,11 @@ static void smack_audit_rule_free(void *vrule) */ static int smack_secid_to_secctx(u32 secid, char **secdata, u32 *seclen) { - char *sp = smack_from_secid(secid); + struct smack_known *skp = smack_from_secid(secid); if (secdata) - *secdata = sp; - *seclen = strlen(sp); + *secdata = skp->smk_known; + *seclen = strlen(skp->smk_known); return 0; } @@ -3845,8 +3874,8 @@ static __init int smack_init(void) if (!security_module_enable(&smack_ops)) return 0; - tsp = new_task_smack(smack_known_floor.smk_known, - smack_known_floor.smk_known, GFP_KERNEL); + tsp = new_task_smack(&smack_known_floor, &smack_known_floor, + GFP_KERNEL); if (tsp == NULL) return -ENOMEM; diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c index 53a08b85bda4..3c79cba5fa4a 100644 --- a/security/smack/smackfs.c +++ b/security/smack/smackfs.c @@ -66,7 +66,7 @@ static DEFINE_MUTEX(smk_netlbladdr_lock); * If it isn't somehow marked, use this. * It can be reset via smackfs/ambient */ -char *smack_net_ambient; +struct smack_known *smack_net_ambient; /* * This is the level in a CIPSO header that indicates a @@ -112,7 +112,7 @@ struct smack_master_list { LIST_HEAD(smack_rule_list); struct smack_parsed_rule { - char *smk_subject; + struct smack_known *smk_subject; char *smk_object; int smk_access1; int smk_access2; @@ -163,9 +163,11 @@ static inline void smack_catset_bit(unsigned int cat, char *catsetp) */ static void smk_netlabel_audit_set(struct netlbl_audit *nap) { + struct smack_known *skp = smk_of_current(); + nap->loginuid = audit_get_loginuid(current); nap->sessionid = audit_get_sessionid(current); - nap->secid = smack_to_secid(smk_of_current()); + nap->secid = skp->smk_secid; } /* @@ -306,7 +308,7 @@ static int smk_fill_rule(const char *subject, const char *object, struct smack_known *skp; if (import) { - rule->smk_subject = smk_import(subject, len); + rule->smk_subject = smk_import_entry(subject, len); if (rule->smk_subject == NULL) return -1; @@ -321,7 +323,7 @@ static int smk_fill_rule(const char *subject, const char *object, kfree(cp); if (skp == NULL) return -1; - rule->smk_subject = skp->smk_known; + rule->smk_subject = skp; cp = smk_parse_smack(object, len); if (cp == NULL) @@ -445,7 +447,6 @@ static ssize_t smk_write_rules_list(struct file *file, const char __user *buf, struct list_head *rule_list, struct mutex *rule_lock, int format) { - struct smack_known *skp; struct smack_parsed_rule *rule; char *data; int datalen; @@ -505,12 +506,10 @@ static ssize_t smk_write_rules_list(struct file *file, const char __user *buf, goto out_free_rule; } - if (rule_list == NULL) { load = 1; - skp = smk_find_entry(rule->smk_subject); - rule_list = &skp->smk_rules; - rule_lock = &skp->smk_rules_lock; + rule_list = &rule->smk_subject->smk_rules; + rule_lock = &rule->smk_subject->smk_rules_lock; } rc = smk_set_access(rule, rule_list, rule_lock, load); @@ -579,13 +578,14 @@ static void smk_rule_show(struct seq_file *s, struct smack_rule *srp, int max) * because you should expect to be able to write * anything you read back. */ - if (strlen(srp->smk_subject) >= max || strlen(srp->smk_object) >= max) + if (strlen(srp->smk_subject->smk_known) >= max || + strlen(srp->smk_object) >= max) return; if (srp->smk_access == 0) return; - seq_printf(s, "%s %s", srp->smk_subject, srp->smk_object); + seq_printf(s, "%s %s", srp->smk_subject->smk_known, srp->smk_object); seq_putc(s, ' '); @@ -738,9 +738,9 @@ static void smk_unlbl_ambient(char *oldambient) __func__, __LINE__, rc); } if (smack_net_ambient == NULL) - smack_net_ambient = smack_known_floor.smk_known; + smack_net_ambient = &smack_known_floor; - rc = netlbl_cfg_unlbl_map_add(smack_net_ambient, PF_INET, + rc = netlbl_cfg_unlbl_map_add(smack_net_ambient->smk_known, PF_INET, NULL, NULL, &nai); if (rc != 0) printk(KERN_WARNING "%s:%d add rc = %d\n", @@ -1535,11 +1535,12 @@ static ssize_t smk_read_ambient(struct file *filp, char __user *buf, */ mutex_lock(&smack_ambient_lock); - asize = strlen(smack_net_ambient) + 1; + asize = strlen(smack_net_ambient->smk_known) + 1; if (cn >= asize) rc = simple_read_from_buffer(buf, cn, ppos, - smack_net_ambient, asize); + smack_net_ambient->smk_known, + asize); else rc = -EINVAL; @@ -1560,8 +1561,8 @@ static ssize_t smk_read_ambient(struct file *filp, char __user *buf, static ssize_t smk_write_ambient(struct file *file, const char __user *buf, size_t count, loff_t *ppos) { + struct smack_known *skp; char *oldambient; - char *smack = NULL; char *data; int rc = count; @@ -1577,16 +1578,16 @@ static ssize_t smk_write_ambient(struct file *file, const char __user *buf, goto out; } - smack = smk_import(data, count); - if (smack == NULL) { + skp = smk_import_entry(data, count); + if (skp == NULL) { rc = -EINVAL; goto out; } mutex_lock(&smack_ambient_lock); - oldambient = smack_net_ambient; - smack_net_ambient = smack; + oldambient = smack_net_ambient->smk_known; + smack_net_ambient = skp; smk_unlbl_ambient(oldambient); mutex_unlock(&smack_ambient_lock); @@ -1645,7 +1646,7 @@ static ssize_t smk_write_onlycap(struct file *file, const char __user *buf, size_t count, loff_t *ppos) { char *data; - char *sp = smk_of_task(current->cred->security); + struct smack_known *skp = smk_of_task(current->cred->security); int rc = count; if (!smack_privileged(CAP_MAC_ADMIN)) @@ -1656,7 +1657,7 @@ static ssize_t smk_write_onlycap(struct file *file, const char __user *buf, * explicitly for clarity. The smk_access() implementation * would use smk_access(smack_onlycap, MAY_WRITE) */ - if (smack_onlycap != NULL && smack_onlycap != sp) + if (smack_onlycap != NULL && smack_onlycap != skp->smk_known) return -EPERM; data = kzalloc(count, GFP_KERNEL); @@ -1866,8 +1867,8 @@ static ssize_t smk_user_access(struct file *file, const char __user *buf, if (res) return -EINVAL; - res = smk_access(rule.smk_subject, rule.smk_object, rule.smk_access1, - NULL); + res = smk_access(rule.smk_subject, rule.smk_object, + rule.smk_access1, NULL); data[0] = res == 0 ? '1' : '0'; data[1] = '\0'; -- cgit v1.2.3-59-g8ed1b From e830b39412ca2bbedd7508243f21c04d57ad543c Mon Sep 17 00:00:00 2001 From: Casey Schaufler Date: Wed, 22 May 2013 18:43:07 -0700 Subject: Smack: Add smkfstransmute mount option Suppliment the smkfsroot mount option with another, smkfstransmute, that does the same thing but also marks the root inode as transmutting. This allows a freshly created filesystem to be mounted with a transmutting heirarchy. Targeted for git://git.gitorious.org/smack-next/kernel.git Signed-off-by: Casey Schaufler --- security/smack/smack.h | 1 + security/smack/smack_lsm.c | 25 ++++++++++++++++++++----- 2 files changed, 21 insertions(+), 5 deletions(-) (limited to 'security') diff --git a/security/smack/smack.h b/security/smack/smack.h index 159f25bfcf45..339614c76e63 100644 --- a/security/smack/smack.h +++ b/security/smack/smack.h @@ -143,6 +143,7 @@ struct smk_port_label { #define SMK_FSFLOOR "smackfsfloor=" #define SMK_FSHAT "smackfshat=" #define SMK_FSROOT "smackfsroot=" +#define SMK_FSTRANS "smackfstransmute=" #define SMACK_CIPSO_OPTION "-CIPSO" diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 3669d9f9824e..6a083303501d 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -261,8 +261,9 @@ static int smack_sb_alloc_security(struct super_block *sb) sbsp->smk_default = smack_known_floor.smk_known; sbsp->smk_floor = smack_known_floor.smk_known; sbsp->smk_hat = smack_known_hat.smk_known; - sbsp->smk_initialized = 0; - + /* + * smk_initialized will be zero from kzalloc. + */ sb->s_security = sbsp; return 0; @@ -306,6 +307,8 @@ static int smack_sb_copy_data(char *orig, char *smackopts) dp = smackopts; else if (strstr(cp, SMK_FSROOT) == cp) dp = smackopts; + else if (strstr(cp, SMK_FSTRANS) == cp) + dp = smackopts; else dp = otheropts; @@ -341,8 +344,9 @@ static int smack_sb_kern_mount(struct super_block *sb, int flags, void *data) char *op; char *commap; char *nsp; + int transmute = 0; - if (sp->smk_initialized != 0) + if (sp->smk_initialized) return 0; sp->smk_initialized = 1; @@ -373,6 +377,13 @@ static int smack_sb_kern_mount(struct super_block *sb, int flags, void *data) nsp = smk_import(op, 0); if (nsp != NULL) sp->smk_root = nsp; + } else if (strncmp(op, SMK_FSTRANS, strlen(SMK_FSTRANS)) == 0) { + op += strlen(SMK_FSTRANS); + nsp = smk_import(op, 0); + if (nsp != NULL) { + sp->smk_root = nsp; + transmute = 1; + } } } @@ -380,11 +391,15 @@ static int smack_sb_kern_mount(struct super_block *sb, int flags, void *data) * Initialize the root inode. */ isp = inode->i_security; - if (isp == NULL) + if (inode->i_security == NULL) { inode->i_security = new_inode_smack(sp->smk_root); - else + isp = inode->i_security; + } else isp->smk_inode = sp->smk_root; + if (transmute) + isp->smk_flags |= SMK_INODE_TRANSMUTE; + return 0; } -- cgit v1.2.3-59-g8ed1b From 8cd77a0bd4b4a7d02c2a6926a69585d8088ee721 Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Mon, 27 May 2013 20:11:27 +0900 Subject: Smack: Fix possible NULL pointer dereference at smk_netlbl_mls() netlbl_secattr_catmap_alloc(GFP_ATOMIC) can return NULL. Signed-off-by: Tetsuo Handa --- security/smack/smack_access.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'security') diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c index 53f2327a592f..6a0377f38620 100644 --- a/security/smack/smack_access.c +++ b/security/smack/smack_access.c @@ -403,6 +403,8 @@ int smk_netlbl_mls(int level, char *catset, struct netlbl_lsm_secattr *sap, sap->flags |= NETLBL_SECATTR_MLS_CAT; sap->attr.mls.lvl = level; sap->attr.mls.cat = netlbl_secattr_catmap_alloc(GFP_ATOMIC); + if (!sap->attr.mls.cat) + return -ENOMEM; sap->attr.mls.cat->startbit = 0; for (cat = 1, cp = catset, byte = 0; byte < len; cp++, byte++) -- cgit v1.2.3-59-g8ed1b From 0fcfee61d63b82c1eefb5b1a914240480f17d63f Mon Sep 17 00:00:00 2001 From: "Passion,Zhao" Date: Mon, 3 Jun 2013 11:42:24 +0800 Subject: Smack: Fix the bug smackcipso can't set CIPSO correctly Bug report: https://tizendev.org/bugs/browse/TDIS-3891 The reason is userspace libsmack only use "smackfs/cipso2" long-label interface, but the code's logical is still for orginal fixed length label. Now update smack_cipso_apply() to support flexible label (<=256 including tailing '\0') There is also a bug in kernel/security/smack/smackfs.c: When smk_set_cipso() parsing the CIPSO setting from userspace, the offset of CIPSO level should be "strlen(label)+1" instead of "strlen(label)" Signed-off-by: Passion,Zhao --- security/smack/smackfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security') diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c index 3c79cba5fa4a..ab167037b2dd 100644 --- a/security/smack/smackfs.c +++ b/security/smack/smackfs.c @@ -881,7 +881,7 @@ static ssize_t smk_set_cipso(struct file *file, const char __user *buf, if (format == SMK_FIXED24_FMT) rule += SMK_LABELLEN; else - rule += strlen(skp->smk_known); + rule += strlen(skp->smk_known) + 1; ret = sscanf(rule, "%d", &maplevel); if (ret != 1 || maplevel > SMACK_CIPSO_MAXLEVEL) -- cgit v1.2.3-59-g8ed1b From d726d8d719b6ac919cc4d5cae73831a2ffe36118 Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Mon, 18 Mar 2013 14:48:02 -0400 Subject: integrity: move integrity_audit_msg() This patch moves the integrity_audit_msg() function and defintion to security/integrity/, the parent directory, renames the 'ima_audit' boot command line option to 'integrity_audit', and fixes the Kconfig help text to reflect the actual code. Changelog: - Fixed ifdef inclusion of integrity_audit_msg() (Fengguang Wu) Signed-off-by: Mimi Zohar --- Documentation/kernel-parameters.txt | 10 +++--- security/integrity/Kconfig | 15 +++++++++ security/integrity/Makefile | 1 + security/integrity/ima/Kconfig | 12 ------- security/integrity/ima/Makefile | 1 - security/integrity/ima/ima.h | 14 -------- security/integrity/ima/ima_audit.c | 64 ------------------------------------ security/integrity/integrity.h | 14 ++++++++ security/integrity/integrity_audit.c | 64 ++++++++++++++++++++++++++++++++++++ 9 files changed, 99 insertions(+), 96 deletions(-) delete mode 100644 security/integrity/ima/ima_audit.c create mode 100644 security/integrity/integrity_audit.c (limited to 'security') diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt index c3bfacb92910..cb5daa1cd605 100644 --- a/Documentation/kernel-parameters.txt +++ b/Documentation/kernel-parameters.txt @@ -1129,11 +1129,6 @@ bytes respectively. Such letter suffixes can also be entirely omitted. The builtin appraise policy appraises all files owned by uid=0. - ima_audit= [IMA] - Format: { "0" | "1" } - 0 -- integrity auditing messages. (Default) - 1 -- enable informational integrity auditing messages. - ima_hash= [IMA] Format: { "sha1" | "md5" } default: "sha1" @@ -1158,6 +1153,11 @@ bytes respectively. Such letter suffixes can also be entirely omitted. inport.irq= [HW] Inport (ATI XL and Microsoft) busmouse driver Format: + integrity_audit=[IMA] + Format: { "0" | "1" } + 0 -- basic integrity auditing messages. (Default) + 1 -- additional integrity auditing messages. + intel_iommu= [DMAR] Intel IOMMU driver (DMAR) option on Enable intel iommu driver. diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig index 4bb3a775a996..245c6d92065b 100644 --- a/security/integrity/Kconfig +++ b/security/integrity/Kconfig @@ -17,6 +17,21 @@ config INTEGRITY_SIGNATURE This is useful for evm and module keyrings, when keys are usually only added from initramfs. +config INTEGRITY_AUDIT + bool "Enables integrity auditing support " + depends on INTEGRITY && AUDIT + default y + help + In addition to enabling integrity auditing support, this + option adds a kernel parameter 'integrity_audit', which + controls the level of integrity auditing messages. + 0 - basic integrity auditing messages (default) + 1 - additional integrity auditing messages + + Additional informational integrity auditing messages would + be enabled by specifying 'integrity_audit=1' on the kernel + command line. + config INTEGRITY_ASYMMETRIC_KEYS boolean "Enable asymmetric keys support" depends on INTEGRITY_SIGNATURE diff --git a/security/integrity/Makefile b/security/integrity/Makefile index ebb6409b3fcb..0f9cffb1f9ad 100644 --- a/security/integrity/Makefile +++ b/security/integrity/Makefile @@ -3,6 +3,7 @@ # obj-$(CONFIG_INTEGRITY) += integrity.o +obj-$(CONFIG_INTEGRITY_AUDIT) += integrity_audit.o obj-$(CONFIG_INTEGRITY_SIGNATURE) += digsig.o obj-$(CONFIG_INTEGRITY_ASYMMETRIC_KEYS) += digsig_asymmetric.o diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig index d232c73647ae..39196abaff0d 100644 --- a/security/integrity/ima/Kconfig +++ b/security/integrity/ima/Kconfig @@ -38,18 +38,6 @@ config IMA_MEASURE_PCR_IDX that IMA uses to maintain the integrity aggregate of the measurement list. If unsure, use the default 10. -config IMA_AUDIT - bool "Enables auditing support" - depends on IMA - depends on AUDIT - default y - help - This option adds a kernel parameter 'ima_audit', which - allows informational auditing messages to be enabled - at boot. If this option is selected, informational integrity - auditing messages can be enabled with 'ima_audit=1' on - the kernel command line. - config IMA_LSM_RULES bool depends on IMA && AUDIT && (SECURITY_SELINUX || SECURITY_SMACK) diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile index 3f2ca6bdc384..56dfee7cbf61 100644 --- a/security/integrity/ima/Makefile +++ b/security/integrity/ima/Makefile @@ -7,5 +7,4 @@ obj-$(CONFIG_IMA) += ima.o ima-y := ima_fs.o ima_queue.o ima_init.o ima_main.o ima_crypto.o ima_api.o \ ima_policy.o -ima-$(CONFIG_IMA_AUDIT) += ima_audit.o ima-$(CONFIG_IMA_APPRAISE) += ima_appraise.o diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index a41c9c18e5e0..b3dd616560f7 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -62,20 +62,6 @@ struct ima_queue_entry { }; extern struct list_head ima_measurements; /* list of all measurements */ -#ifdef CONFIG_IMA_AUDIT -/* declarations */ -void integrity_audit_msg(int audit_msgno, struct inode *inode, - const unsigned char *fname, const char *op, - const char *cause, int result, int info); -#else -static inline void integrity_audit_msg(int audit_msgno, struct inode *inode, - const unsigned char *fname, - const char *op, const char *cause, - int result, int info) -{ -} -#endif - /* Internal IMA function definitions */ int ima_init(void); void ima_cleanup(void); diff --git a/security/integrity/ima/ima_audit.c b/security/integrity/ima/ima_audit.c deleted file mode 100644 index c586faae8fd6..000000000000 --- a/security/integrity/ima/ima_audit.c +++ /dev/null @@ -1,64 +0,0 @@ -/* - * Copyright (C) 2008 IBM Corporation - * Author: Mimi Zohar - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, version 2 of the License. - * - * File: integrity_audit.c - * Audit calls for the integrity subsystem - */ - -#include -#include -#include -#include "ima.h" - -static int ima_audit; - -/* ima_audit_setup - enable informational auditing messages */ -static int __init ima_audit_setup(char *str) -{ - unsigned long audit; - - if (!strict_strtoul(str, 0, &audit)) - ima_audit = audit ? 1 : 0; - return 1; -} -__setup("ima_audit=", ima_audit_setup); - -void integrity_audit_msg(int audit_msgno, struct inode *inode, - const unsigned char *fname, const char *op, - const char *cause, int result, int audit_info) -{ - struct audit_buffer *ab; - - if (!ima_audit && audit_info == 1) /* Skip informational messages */ - return; - - ab = audit_log_start(current->audit_context, GFP_KERNEL, audit_msgno); - audit_log_format(ab, "pid=%d uid=%u auid=%u ses=%u", - current->pid, - from_kuid(&init_user_ns, current_cred()->uid), - from_kuid(&init_user_ns, audit_get_loginuid(current)), - audit_get_sessionid(current)); - audit_log_task_context(ab); - audit_log_format(ab, " op="); - audit_log_string(ab, op); - audit_log_format(ab, " cause="); - audit_log_string(ab, cause); - audit_log_format(ab, " comm="); - audit_log_untrustedstring(ab, current->comm); - if (fname) { - audit_log_format(ab, " name="); - audit_log_untrustedstring(ab, fname); - } - if (inode) { - audit_log_format(ab, " dev="); - audit_log_untrustedstring(ab, inode->i_sb->s_id); - audit_log_format(ab, " ino=%lu", inode->i_ino); - } - audit_log_format(ab, " res=%d", !result); - audit_log_end(ab); -} diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h index 84c37c4db914..c42fb7a70dee 100644 --- a/security/integrity/integrity.h +++ b/security/integrity/integrity.h @@ -113,5 +113,19 @@ static inline int asymmetric_verify(struct key *keyring, const char *sig, } #endif +#ifdef CONFIG_INTEGRITY_AUDIT +/* declarations */ +void integrity_audit_msg(int audit_msgno, struct inode *inode, + const unsigned char *fname, const char *op, + const char *cause, int result, int info); +#else +static inline void integrity_audit_msg(int audit_msgno, struct inode *inode, + const unsigned char *fname, + const char *op, const char *cause, + int result, int info) +{ +} +#endif + /* set during initialization */ extern int iint_initialized; diff --git a/security/integrity/integrity_audit.c b/security/integrity/integrity_audit.c new file mode 100644 index 000000000000..d7efb30404aa --- /dev/null +++ b/security/integrity/integrity_audit.c @@ -0,0 +1,64 @@ +/* + * Copyright (C) 2008 IBM Corporation + * Author: Mimi Zohar + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, version 2 of the License. + * + * File: integrity_audit.c + * Audit calls for the integrity subsystem + */ + +#include +#include +#include +#include "integrity.h" + +static int integrity_audit_info; + +/* ima_audit_setup - enable informational auditing messages */ +static int __init integrity_audit_setup(char *str) +{ + unsigned long audit; + + if (!strict_strtoul(str, 0, &audit)) + integrity_audit_info = audit ? 1 : 0; + return 1; +} +__setup("integrity_audit=", integrity_audit_setup); + +void integrity_audit_msg(int audit_msgno, struct inode *inode, + const unsigned char *fname, const char *op, + const char *cause, int result, int audit_info) +{ + struct audit_buffer *ab; + + if (!integrity_audit_info && audit_info == 1) /* Skip info messages */ + return; + + ab = audit_log_start(current->audit_context, GFP_KERNEL, audit_msgno); + audit_log_format(ab, "pid=%d uid=%u auid=%u ses=%u", + current->pid, + from_kuid(&init_user_ns, current_cred()->uid), + from_kuid(&init_user_ns, audit_get_loginuid(current)), + audit_get_sessionid(current)); + audit_log_task_context(ab); + audit_log_format(ab, " op="); + audit_log_string(ab, op); + audit_log_format(ab, " cause="); + audit_log_string(ab, cause); + audit_log_format(ab, " comm="); + audit_log_untrustedstring(ab, current->comm); + if (fname) { + audit_log_format(ab, " name="); + audit_log_untrustedstring(ab, fname); + } + if (inode) { + audit_log_format(ab, " dev="); + audit_log_untrustedstring(ab, inode->i_sb->s_id); + audit_log_format(ab, " ino=%lu", inode->i_ino); + } + audit_log_format(ab, " res=%d", !result); + audit_log_end(ab); +} -- cgit v1.2.3-59-g8ed1b From 9b97b6cdd420cd62dae972eafaae7494a7670607 Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Thu, 21 Feb 2013 09:31:22 -0500 Subject: evm: audit integrity metadata failures Before modifying an EVM protected extended attribute or any other metadata included in the HMAC calculation, the existing 'security.evm' is verified. This patch adds calls to integrity_audit_msg() to audit integrity metadata failures. Reported-by: Sven Vermeulen Signed-off-by: Mimi Zohar --- security/integrity/evm/evm_main.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) (limited to 'security') diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index cdbde1762189..df0fa451a871 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -16,6 +16,7 @@ #include #include +#include #include #include #include @@ -24,6 +25,9 @@ int evm_initialized; +static char *integrity_status_msg[] = { + "pass", "fail", "no_label", "no_xattrs", "unknown" +}; char *evm_hmac = "hmac(sha1)"; char *evm_hash = "sha1"; int evm_hmac_version = CONFIG_EVM_HMAC_VERSION; @@ -262,9 +266,15 @@ static int evm_protect_xattr(struct dentry *dentry, const char *xattr_name, if ((evm_status == INTEGRITY_PASS) || (evm_status == INTEGRITY_NOXATTRS)) return 0; - return -EPERM; + goto out; } evm_status = evm_verify_current_integrity(dentry); +out: + if (evm_status != INTEGRITY_PASS) + integrity_audit_msg(AUDIT_INTEGRITY_METADATA, dentry->d_inode, + dentry->d_name.name, "appraise_metadata", + integrity_status_msg[evm_status], + -EPERM, 0); return evm_status == INTEGRITY_PASS ? 0 : -EPERM; } @@ -357,6 +367,9 @@ int evm_inode_setattr(struct dentry *dentry, struct iattr *attr) if ((evm_status == INTEGRITY_PASS) || (evm_status == INTEGRITY_NOXATTRS)) return 0; + integrity_audit_msg(AUDIT_INTEGRITY_METADATA, dentry->d_inode, + dentry->d_name.name, "appraise_metadata", + integrity_status_msg[evm_status], -EPERM, 0); return -EPERM; } -- cgit v1.2.3-59-g8ed1b