From d590dca62bb8a315a2356159444de07278a3ad83 Mon Sep 17 00:00:00 2001 From: Richard Guy Briggs Date: Sat, 3 Feb 2018 00:33:11 -0500 Subject: audit: update bugtracker and source URIs Since the Linux Audit project has transitioned completely over to github, update the MAINTAINERS file and the primary audit source file to reflect that reality. Signed-off-by: Richard Guy Briggs Signed-off-by: Paul Moore --- kernel/audit.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'kernel/audit.c') diff --git a/kernel/audit.c b/kernel/audit.c index 227db99b0f19..5c2544984375 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -38,7 +38,8 @@ * 6) Support low-overhead kernel-based filtering to minimize the * information that must be passed to user-space. * - * Example user-space utilities: http://people.redhat.com/sgrubb/audit/ + * Audit userspace, documentation, tests, and bug/issue trackers: + * https://github.com/linux-audit */ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt -- cgit From 23138ead270045f1b3e912e667967b6094244999 Mon Sep 17 00:00:00 2001 From: Richard Guy Briggs Date: Wed, 21 Feb 2018 04:30:07 -0500 Subject: audit: return on memory error to avoid null pointer dereference If there is a memory allocation error when trying to change an audit kernel feature value, the ignored allocation error will trigger a NULL pointer dereference oops on subsequent use of that pointer. Return instead. Passes audit-testsuite. See: https://github.com/linux-audit/audit-kernel/issues/76 Signed-off-by: Richard Guy Briggs [PM: not necessary (other funcs check for NULL), but a good practice] Signed-off-by: Paul Moore --- kernel/audit.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'kernel/audit.c') diff --git a/kernel/audit.c b/kernel/audit.c index 5c2544984375..2de74be7cef5 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -1059,6 +1059,8 @@ static void audit_log_feature_change(int which, u32 old_feature, u32 new_feature return; ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_FEATURE_CHANGE); + if (!ab) + return; audit_log_task_info(ab, current); audit_log_format(ab, " feature=%s old=%u new=%u old_lock=%u new_lock=%u res=%d", audit_feature_names[which], !!old_feature, !!new_feature, -- cgit From ce423631ce1f20564f818e7de6bc0eee0c01badd Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Tue, 20 Feb 2018 09:52:38 -0500 Subject: audit: track the owner of the command mutex ourselves Evidently the __mutex_owner() function was never intended for use outside the core mutex code, so build a thing locking wrapper around the mutex code which allows us to track the mutex owner. One, arguably positive, side effect is that this allows us to hide the audit_cmd_mutex inside of kernel/audit.c behind the lock/unlock functions. Reported-by: Peter Zijlstra Reviewed-by: Richard Guy Briggs Signed-off-by: Paul Moore --- kernel/audit.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 55 insertions(+), 11 deletions(-) (limited to 'kernel/audit.c') diff --git a/kernel/audit.c b/kernel/audit.c index 2de74be7cef5..1a3e75d9a66c 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -181,9 +181,21 @@ static char *audit_feature_names[2] = { "loginuid_immutable", }; - -/* Serialize requests from userspace. */ -DEFINE_MUTEX(audit_cmd_mutex); +/** + * struct audit_ctl_mutex - serialize requests from userspace + * @lock: the mutex used for locking + * @owner: the task which owns the lock + * + * Description: + * This is the lock struct used to ensure we only process userspace requests + * in an orderly fashion. We can't simply use a mutex/lock here because we + * need to track lock ownership so we don't end up blocking the lock owner in + * audit_log_start() or similar. + */ +static struct audit_ctl_mutex { + struct mutex lock; + void *owner; +} audit_cmd_mutex; /* AUDIT_BUFSIZ is the size of the temporary buffer used for formatting * audit records. Since printk uses a 1024 byte buffer, this buffer @@ -227,6 +239,36 @@ int auditd_test_task(struct task_struct *task) return rc; } +/** + * audit_ctl_lock - Take the audit control lock + */ +void audit_ctl_lock(void) +{ + mutex_lock(&audit_cmd_mutex.lock); + audit_cmd_mutex.owner = current; +} + +/** + * audit_ctl_unlock - Drop the audit control lock + */ +void audit_ctl_unlock(void) +{ + audit_cmd_mutex.owner = NULL; + mutex_unlock(&audit_cmd_mutex.lock); +} + +/** + * audit_ctl_owner_current - Test to see if the current task owns the lock + * + * Description: + * Return true if the current task owns the audit control lock, false if it + * doesn't own the lock. + */ +static bool audit_ctl_owner_current(void) +{ + return (current == audit_cmd_mutex.owner); +} + /** * auditd_pid_vnr - Return the auditd PID relative to the namespace * @@ -861,8 +903,8 @@ int audit_send_list(void *_dest) struct sock *sk = audit_get_sk(dest->net); /* wait for parent to finish and send an ACK */ - mutex_lock(&audit_cmd_mutex); - mutex_unlock(&audit_cmd_mutex); + audit_ctl_lock(); + audit_ctl_unlock(); while ((skb = __skb_dequeue(&dest->q)) != NULL) netlink_unicast(sk, skb, dest->portid, 0); @@ -903,8 +945,8 @@ static int audit_send_reply_thread(void *arg) struct audit_reply *reply = (struct audit_reply *)arg; struct sock *sk = audit_get_sk(reply->net); - mutex_lock(&audit_cmd_mutex); - mutex_unlock(&audit_cmd_mutex); + audit_ctl_lock(); + audit_ctl_unlock(); /* Ignore failure. It'll only happen if the sender goes away, because our timeout is set to infinite. */ @@ -1469,7 +1511,7 @@ static void audit_receive(struct sk_buff *skb) nlh = nlmsg_hdr(skb); len = skb->len; - mutex_lock(&audit_cmd_mutex); + audit_ctl_lock(); while (nlmsg_ok(nlh, len)) { err = audit_receive_msg(skb, nlh); /* if err or if this message says it wants a response */ @@ -1478,7 +1520,7 @@ static void audit_receive(struct sk_buff *skb) nlh = nlmsg_next(nlh, &len); } - mutex_unlock(&audit_cmd_mutex); + audit_ctl_unlock(); } /* Run custom bind function on netlink socket group connect or bind requests. */ @@ -1550,6 +1592,9 @@ static int __init audit_init(void) for (i = 0; i < AUDIT_INODE_BUCKETS; i++) INIT_LIST_HEAD(&audit_inode_hash[i]); + mutex_init(&audit_cmd_mutex.lock); + audit_cmd_mutex.owner = NULL; + pr_info("initializing netlink subsys (%s)\n", audit_default ? "enabled" : "disabled"); register_pernet_subsys(&audit_net_ops); @@ -1713,8 +1758,7 @@ struct audit_buffer *audit_log_start(struct audit_context *ctx, gfp_t gfp_mask, * using a PID anchored in the caller's namespace * 2. generator holding the audit_cmd_mutex - we don't want to block * while holding the mutex */ - if (!(auditd_test_task(current) || - (current == __mutex_owner(&audit_cmd_mutex)))) { + if (!(auditd_test_task(current) || audit_ctl_owner_current())) { long stime = audit_backlog_wait_time; while (audit_backlog_limit && -- cgit From 11dd2666375e191757dd4271d5020820c6d0e4a5 Mon Sep 17 00:00:00 2001 From: Greg Edwards Date: Mon, 5 Mar 2018 15:05:20 -0700 Subject: audit: do not panic on invalid boot parameter If you pass in an invalid audit boot parameter value, e.g. "audit=off", the kernel panics very early in boot before the regular console is initialized. Unless you have earlyprintk enabled, there is no indication of what the problem is on the console. Convert the panic() calls to pr_err(), and leave auditing enabled if an invalid parameter value was passed in. Modify the parameter to also accept "on" or "off" as valid values, and update the documentation accordingly. Signed-off-by: Greg Edwards Signed-off-by: Paul Moore --- kernel/audit.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) (limited to 'kernel/audit.c') diff --git a/kernel/audit.c b/kernel/audit.c index 1a3e75d9a66c..69ef8de69f03 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -1615,19 +1615,26 @@ static int __init audit_init(void) } postcore_initcall(audit_init); -/* Process kernel command-line parameter at boot time. audit=0 or audit=1. */ +/* + * Process kernel command-line parameter at boot time. + * audit={0|off} or audit={1|on}. + */ static int __init audit_enable(char *str) { - long val; - - if (kstrtol(str, 0, &val)) - panic("audit: invalid 'audit' parameter value (%s)\n", str); - audit_default = (val ? AUDIT_ON : AUDIT_OFF); + if (!strcasecmp(str, "off") || !strcmp(str, "0")) + audit_default = AUDIT_OFF; + else if (!strcasecmp(str, "on") || !strcmp(str, "1")) + audit_default = AUDIT_ON; + else { + pr_err("audit: invalid 'audit' parameter value (%s)\n", str); + audit_default = AUDIT_ON; + } if (audit_default == AUDIT_OFF) audit_initialized = AUDIT_DISABLED; if (audit_set_enabled(audit_default)) - panic("audit: error setting audit state (%d)\n", audit_default); + pr_err("audit: error setting audit state (%d)\n", + audit_default); pr_info("%s\n", audit_default ? "enabled (after initialization)" : "disabled (until reboot)"); -- cgit From 15564ff0a16e2d994e78d62c23d227ff182ee864 Mon Sep 17 00:00:00 2001 From: Richard Guy Briggs Date: Wed, 14 Feb 2018 11:18:21 -0500 Subject: audit: make ANOM_LINK obey audit_enabled and audit_dummy_context Audit link denied events emit disjointed records when audit is disabled. No records should be emitted when audit is disabled. See: https://github.com/linux-audit/audit-kernel/issues/21 Signed-off-by: Richard Guy Briggs Signed-off-by: Paul Moore --- kernel/audit.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'kernel/audit.c') diff --git a/kernel/audit.c b/kernel/audit.c index 69ef8de69f03..46cd8f66af17 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -2315,6 +2315,9 @@ void audit_log_link_denied(const char *operation, const struct path *link) struct audit_buffer *ab; struct audit_names *name; + if (!audit_enabled || audit_dummy_context()) + return; + name = kzalloc(sizeof(*name), GFP_NOFS); if (!name) return; -- cgit From 45b578fe4c3cade6f4ca1fc934ce199afd857edc Mon Sep 17 00:00:00 2001 From: Richard Guy Briggs Date: Wed, 14 Feb 2018 11:18:22 -0500 Subject: audit: link denied should not directly generate PATH record Audit link denied events generate duplicate PATH records which disagree in different ways from symlink and hardlink denials. audit_log_link_denied() should not directly generate PATH records. See: https://github.com/linux-audit/audit-kernel/issues/21 Signed-off-by: Richard Guy Briggs Signed-off-by: Paul Moore --- kernel/audit.c | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) (limited to 'kernel/audit.c') diff --git a/kernel/audit.c b/kernel/audit.c index 46cd8f66af17..3f2f143edadf 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -2313,31 +2313,19 @@ EXPORT_SYMBOL(audit_log_task_info); void audit_log_link_denied(const char *operation, const struct path *link) { struct audit_buffer *ab; - struct audit_names *name; if (!audit_enabled || audit_dummy_context()) return; - name = kzalloc(sizeof(*name), GFP_NOFS); - if (!name) - return; - /* Generate AUDIT_ANOM_LINK with subject, operation, outcome. */ ab = audit_log_start(current->audit_context, GFP_KERNEL, AUDIT_ANOM_LINK); if (!ab) - goto out; + return; audit_log_format(ab, "op=%s", operation); audit_log_task_info(ab, current); audit_log_format(ab, " res=0"); audit_log_end(ab); - - /* Generate AUDIT_PATH record with object. */ - name->type = AUDIT_TYPE_NORMAL; - audit_copy_inode(name, link->dentry, d_backing_inode(link->dentry)); - audit_log_name(current->audit_context, name, link, 0, NULL); -out: - kfree(name); } /** -- cgit From 94b9d9b7a14cbb1640868d53b27f403ed2e5b4a9 Mon Sep 17 00:00:00 2001 From: Richard Guy Briggs Date: Wed, 21 Mar 2018 04:42:20 -0400 Subject: audit: remove path param from link denied function In commit 45b578fe4c3cade6f4ca1fc934ce199afd857edc ("audit: link denied should not directly generate PATH record") the need for the struct path *link parameter was removed. Remove the now useless struct path argument. Signed-off-by: Richard Guy Briggs Signed-off-by: Paul Moore --- kernel/audit.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'kernel/audit.c') diff --git a/kernel/audit.c b/kernel/audit.c index 3f2f143edadf..e8bf8d78ac4a 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -2308,9 +2308,8 @@ EXPORT_SYMBOL(audit_log_task_info); /** * audit_log_link_denied - report a link restriction denial * @operation: specific link operation - * @link: the path that triggered the restriction */ -void audit_log_link_denied(const char *operation, const struct path *link) +void audit_log_link_denied(const char *operation) { struct audit_buffer *ab; -- cgit