From 32a1dbaece7e37cea415e03cd426172249aa859e Mon Sep 17 00:00:00 2001 From: Richard Guy Briggs Date: Wed, 4 Nov 2015 08:23:50 -0500 Subject: audit: try harder to send to auditd upon netlink failure There are several reports of the kernel losing contact with auditd when it is, in fact, still running. When this happens, kernel syslogs show: "audit: *NO* daemon at audit_pid=" although auditd is still running, and is apparently happy, listening on the netlink socket. The pid in the "*NO* daemon" message matches the pid of the running auditd process. Restarting auditd solves this. The problem appears to happen randomly, and doesn't seem to be strongly correlated to the rate of audit events being logged. The problem happens fairly regularly (every few days), but not yet reproduced to order. On production kernels, BUG_ON() is a no-op, so any error will trigger this. Commit 34eab0a7cd45 ("audit: prevent an older auditd shutdown from orphaning a newer auditd startup") eliminates one possible cause. This isn't the case here, since the PID in the error message and the PID of the running auditd match. The primary expected cause of error here is -ECONNREFUSED when the audit daemon goes away, when netlink_getsockbyportid() can't find the auditd portid entry in the netlink audit table (or there is no receive function). If -EPERM is returned, that situation isn't likely to be resolved in a timely fashion without administrator intervention. In both cases, reset the audit_pid. This does not rule out a race condition. SELinux is expected to return zero since this isn't an INET or INET6 socket. Other LSMs may have other return codes. Log the error code for better diagnosis in the future. In the case of -ENOMEM, the situation could be temporary, based on local or general availability of buffers. -EAGAIN should never happen since the netlink audit (kernel) socket is set to MAX_SCHEDULE_TIMEOUT. -ERESTARTSYS and -EINTR are not expected since this kernel thread is not expected to receive signals. In these cases (or any other unexpected ones for now), report the error and re-schedule the thread, retrying up to 5 times. v2: Removed BUG_ON(). Moved comma in pr_*() statements. Removed audit_strerror() text. Reported-by: Vipin Rathor Reported-by: Signed-off-by: Richard Guy Briggs [PM: applied rgb's fixup patch to correct audit_log_lost() format issues] Signed-off-by: Paul Moore --- kernel/audit.c | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) (limited to 'kernel/audit.c') diff --git a/kernel/audit.c b/kernel/audit.c index 662c007635fb..c4c98d87456f 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -407,16 +407,33 @@ static void audit_printk_skb(struct sk_buff *skb) static void kauditd_send_skb(struct sk_buff *skb) { int err; + int attempts = 0; +#define AUDITD_RETRIES 5 + +restart: /* take a reference in case we can't send it and we want to hold it */ skb_get(skb); err = netlink_unicast(audit_sock, skb, audit_nlk_portid, 0); if (err < 0) { - BUG_ON(err != -ECONNREFUSED); /* Shouldn't happen */ + pr_err("netlink_unicast sending to audit_pid=%d returned error: %d\n", + audit_pid, err); if (audit_pid) { - pr_err("*NO* daemon at audit_pid=%d\n", audit_pid); - audit_log_lost("auditd disappeared"); - audit_pid = 0; - audit_sock = NULL; + if (err == -ECONNREFUSED || err == -EPERM + || ++attempts >= AUDITD_RETRIES) { + char s[32]; + + snprintf(s, sizeof(s), "audit_pid=%d reset", audit_pid); + audit_log_lost(s); + audit_pid = 0; + audit_sock = NULL; + } else { + pr_warn("re-scheduling(#%d) write to audit_pid=%d\n", + attempts, audit_pid); + set_current_state(TASK_INTERRUPTIBLE); + schedule(); + __set_current_state(TASK_RUNNING); + goto restart; + } } /* we might get lucky and get this in the next auditd */ audit_hold_skb(skb); -- cgit From 9fcf836b215ca5685030ecab3e35ecc14ee3bcfb Mon Sep 17 00:00:00 2001 From: Yaowei Bai Date: Wed, 4 Nov 2015 08:23:51 -0500 Subject: audit: audit_string_contains_control can be boolean This patch makes audit_string_contains_control return bool to improve readability due to this particular function only using either one or zero as its return value. Signed-off-by: Yaowei Bai [PM: tweaked subject line] Signed-off-by: Paul Moore --- kernel/audit.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'kernel/audit.c') diff --git a/kernel/audit.c b/kernel/audit.c index c4c98d87456f..648036f7690d 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -1583,14 +1583,14 @@ void audit_log_n_string(struct audit_buffer *ab, const char *string, * @string: string to be checked * @len: max length of the string to check */ -int audit_string_contains_control(const char *string, size_t len) +bool audit_string_contains_control(const char *string, size_t len) { const unsigned char *p; for (p = string; p < (const unsigned char *)string + len; p++) { if (*p == '"' || *p < 0x21 || *p > 0x7e) - return 1; + return true; } - return 0; + return false; } /** -- cgit From c5ea6efda6ff0fd591d6b7a2e1ba086b196dd864 Mon Sep 17 00:00:00 2001 From: Saurabh Sengar Date: Wed, 4 Nov 2015 08:23:52 -0500 Subject: audit: removing unused variable Variable rc in not required as it is just used for unchanged for return, and return is always 0 in the function. Signed-off-by: Saurabh Sengar [PM: fixed spelling errors in description] Signed-off-by: Paul Moore --- kernel/audit.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'kernel/audit.c') diff --git a/kernel/audit.c b/kernel/audit.c index 648036f7690d..85570f348ccf 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -703,23 +703,22 @@ static int audit_netlink_ok(struct sk_buff *skb, u16 msg_type) static int audit_log_common_recv_msg(struct audit_buffer **ab, u16 msg_type) { - int rc = 0; uid_t uid = from_kuid(&init_user_ns, current_uid()); pid_t pid = task_tgid_nr(current); if (!audit_enabled && msg_type != AUDIT_USER_AVC) { *ab = NULL; - return rc; + return 0; } *ab = audit_log_start(NULL, GFP_KERNEL, msg_type); if (unlikely(!*ab)) - return rc; + return 0; audit_log_format(*ab, "pid=%d uid=%u", pid, uid); audit_log_session_info(*ab); audit_log_task_context(*ab); - return rc; + return 0; } int is_audit_feature_set(int i) -- cgit From 233a68667cf4c134d07ef7e22bdd77786b5c7360 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Wed, 4 Nov 2015 08:23:52 -0500 Subject: audit: make audit_log_common_recv_msg() a void function It always returns zero and no one is checking the return value. Signed-off-by: Paul Moore --- kernel/audit.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) (limited to 'kernel/audit.c') diff --git a/kernel/audit.c b/kernel/audit.c index 85570f348ccf..8a056a32ded7 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -701,24 +701,22 @@ static int audit_netlink_ok(struct sk_buff *skb, u16 msg_type) return err; } -static int audit_log_common_recv_msg(struct audit_buffer **ab, u16 msg_type) +static void audit_log_common_recv_msg(struct audit_buffer **ab, u16 msg_type) { uid_t uid = from_kuid(&init_user_ns, current_uid()); pid_t pid = task_tgid_nr(current); if (!audit_enabled && msg_type != AUDIT_USER_AVC) { *ab = NULL; - return 0; + return; } *ab = audit_log_start(NULL, GFP_KERNEL, msg_type); if (unlikely(!*ab)) - return 0; + return; audit_log_format(*ab, "pid=%d uid=%u", pid, uid); audit_log_session_info(*ab); audit_log_task_context(*ab); - - return 0; } int is_audit_feature_set(int i) -- cgit