From b7a84deaf8d1b0e62b437a290a40d6380975f126 Mon Sep 17 00:00:00 2001 From: Nicholas Mc Guire Date: Tue, 2 May 2017 10:16:03 -0400 Subject: [PATCH 01/14] audit: remove unnecessary semicolon in audit_field_valid() The excess ; after the closing parenthesis is just code-noise it has no and can be removed. Signed-off-by: Nicholas Mc Guire [PM: tweak subject line] Signed-off-by: Paul Moore --- kernel/auditfilter.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c index 880519d6cf2a..239d11c3122c 100644 --- a/kernel/auditfilter.c +++ b/kernel/auditfilter.c @@ -338,7 +338,7 @@ static int audit_field_valid(struct audit_entry *entry, struct audit_field *f) entry->rule.listnr != AUDIT_FILTER_USER) return -EINVAL; break; - }; + } switch(f->type) { default: @@ -412,7 +412,7 @@ static int audit_field_valid(struct audit_entry *entry, struct audit_field *f) if (entry->rule.listnr != AUDIT_FILTER_EXIT) return -EINVAL; break; - }; + } return 0; } From b5239fba69949a44290d4af517fc1c2eff3e36f6 Mon Sep 17 00:00:00 2001 From: Nicholas Mc Guire Date: Tue, 2 May 2017 10:16:04 -0400 Subject: [PATCH 02/14] audit: remove unnecessary semicolon in audit_mark_handle_event() The excess ; after the closing parenthesis is just code-noise it has no and can be removed. Signed-off-by: Nicholas Mc Guire [PM: tweaked subject line] Signed-off-by: Paul Moore --- kernel/audit_fsnotify.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/audit_fsnotify.c b/kernel/audit_fsnotify.c index 7ea57e516029..b16a5bdcea0d 100644 --- a/kernel/audit_fsnotify.c +++ b/kernel/audit_fsnotify.c @@ -187,7 +187,7 @@ static int audit_mark_handle_event(struct fsnotify_group *group, default: BUG(); return 0; - }; + } if (mask & (FS_CREATE|FS_MOVED_TO|FS_DELETE|FS_MOVED_FROM)) { if (audit_compare_dname_path(dname, audit_mark->path, AUDIT_NAME_FULL)) From 9aab4f4ea7a4ca80ec3e0269ce2eb71a24f6fef9 Mon Sep 17 00:00:00 2001 From: Nicholas Mc Guire Date: Tue, 2 May 2017 10:16:04 -0400 Subject: [PATCH 03/14] audit: remove unnecessary semicolon in audit_watch_handle_event() The excess ; after the closing parenthesis is just code-noise it has no and can be removed. Signed-off-by: Nicholas Mc Guire [PM: tweaked subject line] Signed-off-by: Paul Moore --- kernel/audit_watch.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c index f79e4658433d..e293165dd063 100644 --- a/kernel/audit_watch.c +++ b/kernel/audit_watch.c @@ -492,7 +492,7 @@ static int audit_watch_handle_event(struct fsnotify_group *group, BUG(); inode = NULL; break; - }; + } if (mask & (FS_CREATE|FS_MOVED_TO) && inode) audit_update_watch(parent, dname, inode->i_sb->s_dev, inode->i_ino, 0); From f6276ac95bde4312251535904af32b1de9d54949 Mon Sep 17 00:00:00 2001 From: Richard Guy Briggs Date: Tue, 2 May 2017 10:16:04 -0400 Subject: [PATCH 04/14] audit: log module name on delete_module When a sysadmin wishes to monitor module unloading with a syscall rule such as: -a always,exit -F arch=x86_64 -S delete_module -F key=mod-unload the SYSCALL record doesn't tell us what module was requested for unloading. Use the new KERN_MODULE auxiliary record to record it. The SYSCALL record result code will list the return code. See: https://github.com/linux-audit/audit-kernel/issues/37 https://github.com/linux-audit/audit-kernel/issues/7 https://github.com/linux-audit/audit-kernel/wiki/RFE-Module-Load-Record-Format Signed-off-by: Richard Guy Briggs Acked-by: Jessica Yu Signed-off-by: Paul Moore --- kernel/module.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/kernel/module.c b/kernel/module.c index 7eba6dea4f41..23224d8ba00d 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -947,6 +947,8 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user, return -EFAULT; name[MODULE_NAME_LEN-1] = '\0'; + audit_log_kern_module(name); + if (mutex_lock_interruptible(&module_mutex) != 0) return -EINTR; From 0cb88b6ff054ccfa30e0fd7f7b42ee9f088db432 Mon Sep 17 00:00:00 2001 From: Richard Guy Briggs Date: Tue, 2 May 2017 10:16:04 -0400 Subject: [PATCH 05/14] netfilter: use consistent ipv4 network offset in xt_AUDIT Even though the skb->data pointer has been moved from the link layer header to the network layer header, use the same method to calculate the offset in ipv4 and ipv6 routines. Signed-off-by: Richard Guy Briggs [PM: munged subject line] Signed-off-by: Paul Moore --- net/netfilter/xt_AUDIT.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/netfilter/xt_AUDIT.c b/net/netfilter/xt_AUDIT.c index 19247a17e511..5181f69ec9bf 100644 --- a/net/netfilter/xt_AUDIT.c +++ b/net/netfilter/xt_AUDIT.c @@ -76,7 +76,7 @@ static void audit_ip4(struct audit_buffer *ab, struct sk_buff *skb) struct iphdr _iph; const struct iphdr *ih; - ih = skb_header_pointer(skb, 0, sizeof(_iph), &_iph); + ih = skb_header_pointer(skb, skb_network_offset(skb), sizeof(_iph), &_iph); if (!ih) { audit_log_format(ab, " truncated=1"); return; From 2173c519d5e912a6e2934bb04255fcd36c1591c8 Mon Sep 17 00:00:00 2001 From: Richard Guy Briggs Date: Tue, 2 May 2017 10:16:04 -0400 Subject: [PATCH 06/14] audit: normalize NETFILTER_PKT Eliminate flipping in and out of message fields, dropping fields in the process. Sample raw message format IPv4 UDP: type=NETFILTER_PKT msg=audit(1487874761.386:228): mark=0xae8a2732 saddr=127.0.0.1 daddr=127.0.0.1 proto=17^] Sample raw message format IPv6 ICMP6: type=NETFILTER_PKT msg=audit(1487874761.381:227): mark=0x223894b7 saddr=::1 daddr=::1 proto=58^] Issue: https://github.com/linux-audit/audit-kernel/issues/11 Test case: https://github.com/linux-audit/audit-testsuite/issues/43 Signed-off-by: Richard Guy Briggs Signed-off-by: Paul Moore --- net/netfilter/xt_AUDIT.c | 126 +++++++++------------------------------ 1 file changed, 28 insertions(+), 98 deletions(-) diff --git a/net/netfilter/xt_AUDIT.c b/net/netfilter/xt_AUDIT.c index 5181f69ec9bf..c502419d6306 100644 --- a/net/netfilter/xt_AUDIT.c +++ b/net/netfilter/xt_AUDIT.c @@ -31,146 +31,76 @@ MODULE_ALIAS("ip6t_AUDIT"); MODULE_ALIAS("ebt_AUDIT"); MODULE_ALIAS("arpt_AUDIT"); -static void audit_proto(struct audit_buffer *ab, struct sk_buff *skb, - unsigned int proto, unsigned int offset) -{ - switch (proto) { - case IPPROTO_TCP: - case IPPROTO_UDP: - case IPPROTO_UDPLITE: { - const __be16 *pptr; - __be16 _ports[2]; - - pptr = skb_header_pointer(skb, offset, sizeof(_ports), _ports); - if (pptr == NULL) { - audit_log_format(ab, " truncated=1"); - return; - } - - audit_log_format(ab, " sport=%hu dport=%hu", - ntohs(pptr[0]), ntohs(pptr[1])); - } - break; - - case IPPROTO_ICMP: - case IPPROTO_ICMPV6: { - const u8 *iptr; - u8 _ih[2]; - - iptr = skb_header_pointer(skb, offset, sizeof(_ih), &_ih); - if (iptr == NULL) { - audit_log_format(ab, " truncated=1"); - return; - } - - audit_log_format(ab, " icmptype=%hhu icmpcode=%hhu", - iptr[0], iptr[1]); - - } - break; - } -} - -static void audit_ip4(struct audit_buffer *ab, struct sk_buff *skb) +static bool audit_ip4(struct audit_buffer *ab, struct sk_buff *skb) { struct iphdr _iph; const struct iphdr *ih; ih = skb_header_pointer(skb, skb_network_offset(skb), sizeof(_iph), &_iph); - if (!ih) { - audit_log_format(ab, " truncated=1"); - return; - } + if (!ih) + return false; - audit_log_format(ab, " saddr=%pI4 daddr=%pI4 ipid=%hu proto=%hhu", - &ih->saddr, &ih->daddr, ntohs(ih->id), ih->protocol); + audit_log_format(ab, " saddr=%pI4 daddr=%pI4 proto=%hhu", + &ih->saddr, &ih->daddr, ih->protocol); - if (ntohs(ih->frag_off) & IP_OFFSET) { - audit_log_format(ab, " frag=1"); - return; - } - - audit_proto(ab, skb, ih->protocol, ih->ihl * 4); + return true; } -static void audit_ip6(struct audit_buffer *ab, struct sk_buff *skb) +static bool audit_ip6(struct audit_buffer *ab, struct sk_buff *skb) { struct ipv6hdr _ip6h; const struct ipv6hdr *ih; u8 nexthdr; __be16 frag_off; - int offset; ih = skb_header_pointer(skb, skb_network_offset(skb), sizeof(_ip6h), &_ip6h); - if (!ih) { - audit_log_format(ab, " truncated=1"); - return; - } + if (!ih) + return false; nexthdr = ih->nexthdr; - offset = ipv6_skip_exthdr(skb, skb_network_offset(skb) + sizeof(_ip6h), - &nexthdr, &frag_off); + ipv6_skip_exthdr(skb, skb_network_offset(skb) + sizeof(_ip6h), &nexthdr, &frag_off); audit_log_format(ab, " saddr=%pI6c daddr=%pI6c proto=%hhu", &ih->saddr, &ih->daddr, nexthdr); - if (offset) - audit_proto(ab, skb, nexthdr, offset); + return true; } static unsigned int audit_tg(struct sk_buff *skb, const struct xt_action_param *par) { - const struct xt_audit_info *info = par->targinfo; struct audit_buffer *ab; + int fam = -1; if (audit_enabled == 0) goto errout; - ab = audit_log_start(NULL, GFP_ATOMIC, AUDIT_NETFILTER_PKT); if (ab == NULL) goto errout; - audit_log_format(ab, "action=%hhu hook=%u len=%u inif=%s outif=%s", - info->type, xt_hooknum(par), skb->len, - xt_in(par) ? xt_inname(par) : "?", - xt_out(par) ? xt_outname(par) : "?"); - - if (skb->mark) - audit_log_format(ab, " mark=%#x", skb->mark); - - if (skb->dev && skb->dev->type == ARPHRD_ETHER) { - audit_log_format(ab, " smac=%pM dmac=%pM macproto=0x%04x", - eth_hdr(skb)->h_source, eth_hdr(skb)->h_dest, - ntohs(eth_hdr(skb)->h_proto)); - - if (xt_family(par) == NFPROTO_BRIDGE) { - switch (eth_hdr(skb)->h_proto) { - case htons(ETH_P_IP): - audit_ip4(ab, skb); - break; - - case htons(ETH_P_IPV6): - audit_ip6(ab, skb); - break; - } - } - } + audit_log_format(ab, "mark=%#x", skb->mark); switch (xt_family(par)) { - case NFPROTO_IPV4: - audit_ip4(ab, skb); + case NFPROTO_BRIDGE: + switch (eth_hdr(skb)->h_proto) { + case htons(ETH_P_IP): + fam = audit_ip4(ab, skb) ? NFPROTO_IPV4 : -1; + break; + case htons(ETH_P_IPV6): + fam = audit_ip6(ab, skb) ? NFPROTO_IPV6 : -1; + break; + } + break; + case NFPROTO_IPV4: + fam = audit_ip4(ab, skb) ? NFPROTO_IPV4 : -1; break; - case NFPROTO_IPV6: - audit_ip6(ab, skb); + fam = audit_ip6(ab, skb) ? NFPROTO_IPV6 : -1; break; } -#ifdef CONFIG_NETWORK_SECMARK - if (skb->secmark) - audit_log_secctx(ab, skb->secmark); -#endif + if (fam == -1) + audit_log_format(ab, " saddr=? daddr=? proto=-1"); audit_log_end(ab); From 9d2378f8c8f1a3fcfab681fd90c139d90dca7b69 Mon Sep 17 00:00:00 2001 From: Elena Reshetova Date: Tue, 2 May 2017 10:16:04 -0400 Subject: [PATCH 07/14] audit: convert audit_tree.count from atomic_t to refcount_t refcount_t type and corresponding API should be used instead of atomic_t when the variable is used as a reference counter. This allows to avoid accidental refcounter overflows that might lead to use-after-free situations. Signed-off-by: Elena Reshetova Signed-off-by: Hans Liljestrand Signed-off-by: Kees Cook Signed-off-by: David Windsor [PM: fix subject line, add #include] Signed-off-by: Paul Moore --- kernel/audit_tree.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c index 7b44195da81b..5cfd1ea18de0 100644 --- a/kernel/audit_tree.c +++ b/kernel/audit_tree.c @@ -3,13 +3,14 @@ #include #include #include +#include #include struct audit_tree; struct audit_chunk; struct audit_tree { - atomic_t count; + refcount_t count; int goner; struct audit_chunk *root; struct list_head chunks; @@ -77,7 +78,7 @@ static struct audit_tree *alloc_tree(const char *s) tree = kmalloc(sizeof(struct audit_tree) + strlen(s) + 1, GFP_KERNEL); if (tree) { - atomic_set(&tree->count, 1); + refcount_set(&tree->count, 1); tree->goner = 0; INIT_LIST_HEAD(&tree->chunks); INIT_LIST_HEAD(&tree->rules); @@ -91,12 +92,12 @@ static struct audit_tree *alloc_tree(const char *s) static inline void get_tree(struct audit_tree *tree) { - atomic_inc(&tree->count); + refcount_inc(&tree->count); } static inline void put_tree(struct audit_tree *tree) { - if (atomic_dec_and_test(&tree->count)) + if (refcount_dec_and_test(&tree->count)) kfree_rcu(tree, head); } From bd120ded6a6af61ad342a8a95b36b64bd1e2f9e6 Mon Sep 17 00:00:00 2001 From: Elena Reshetova Date: Tue, 2 May 2017 10:16:05 -0400 Subject: [PATCH 08/14] audit: convert audit_watch.count from atomic_t to refcount_t refcount_t type and corresponding API should be used instead of atomic_t when the variable is used as a reference counter. This allows to avoid accidental refcounter overflows that might lead to use-after-free situations. Signed-off-by: Elena Reshetova Signed-off-by: Hans Liljestrand Signed-off-by: Kees Cook Signed-off-by: David Windsor [PM: fix subject line, add #include] Signed-off-by: Paul Moore --- kernel/audit_watch.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c index e293165dd063..e0656bd63036 100644 --- a/kernel/audit_watch.c +++ b/kernel/audit_watch.c @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -46,7 +47,7 @@ */ struct audit_watch { - atomic_t count; /* reference count */ + refcount_t count; /* reference count */ dev_t dev; /* associated superblock device */ char *path; /* insertion path */ unsigned long ino; /* associated inode number */ @@ -111,12 +112,12 @@ static inline struct audit_parent *audit_find_parent(struct inode *inode) void audit_get_watch(struct audit_watch *watch) { - atomic_inc(&watch->count); + refcount_inc(&watch->count); } void audit_put_watch(struct audit_watch *watch) { - if (atomic_dec_and_test(&watch->count)) { + if (refcount_dec_and_test(&watch->count)) { WARN_ON(watch->parent); WARN_ON(!list_empty(&watch->rules)); kfree(watch->path); @@ -178,7 +179,7 @@ static struct audit_watch *audit_init_watch(char *path) return ERR_PTR(-ENOMEM); INIT_LIST_HEAD(&watch->rules); - atomic_set(&watch->count, 1); + refcount_set(&watch->count, 1); watch->path = path; watch->dev = AUDIT_DEV_UNSET; watch->ino = AUDIT_INO_UNSET; From a9d1620877748375cf60b43ef3fa5f61ab6d9f24 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Tue, 2 May 2017 10:16:05 -0400 Subject: [PATCH 09/14] audit: combine audit_receive() and audit_receive_skb() There is no reason to have both of these functions, combine the two. Signed-off-by: Paul Moore Reviewed-by: Richard Guy Briggs --- kernel/audit.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/kernel/audit.c b/kernel/audit.c index a871bf80fde1..eff602c1aa79 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -1378,11 +1378,14 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh) return err < 0 ? err : 0; } -/* - * Get message from skb. Each message is processed by audit_receive_msg. - * Malformed skbs with wrong length are discarded silently. +/** + * audit_receive - receive messages from a netlink control socket + * @skb: the message buffer + * + * Parse the provided skb and deal with any messages that may be present, + * malformed skbs are discarded. */ -static void audit_receive_skb(struct sk_buff *skb) +static void audit_receive(struct sk_buff *skb) { struct nlmsghdr *nlh; /* @@ -1395,6 +1398,7 @@ static void audit_receive_skb(struct sk_buff *skb) nlh = nlmsg_hdr(skb); len = skb->len; + mutex_lock(&audit_cmd_mutex); while (nlmsg_ok(nlh, len)) { err = audit_receive_msg(skb, nlh); /* if err or if this message says it wants a response */ @@ -1403,13 +1407,6 @@ static void audit_receive_skb(struct sk_buff *skb) nlh = nlmsg_next(nlh, &len); } -} - -/* Receive messages from netlink socket. */ -static void audit_receive(struct sk_buff *skb) -{ - mutex_lock(&audit_cmd_mutex); - audit_receive_skb(skb); mutex_unlock(&audit_cmd_mutex); } From 45a0642b4d021a2f50d5db9c191b5bfe60bfa1c7 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Tue, 2 May 2017 10:16:05 -0400 Subject: [PATCH 10/14] audit: kernel generated netlink traffic should have a portid of 0 We were setting the portid incorrectly in the netlink message headers, fix that to always be 0 (nlmsg_pid = 0). Signed-off-by: Paul Moore Reviewed-by: Richard Guy Briggs --- include/linux/audit.h | 3 +-- kernel/audit.c | 23 ++++++----------------- kernel/audit.h | 3 +-- kernel/auditfilter.c | 14 ++++++-------- 4 files changed, 14 insertions(+), 29 deletions(-) diff --git a/include/linux/audit.h b/include/linux/audit.h index 504e784b7ffa..cc0497c39472 100644 --- a/include/linux/audit.h +++ b/include/linux/audit.h @@ -163,8 +163,7 @@ extern void audit_log_task_info(struct audit_buffer *ab, extern int audit_update_lsm_rules(void); /* Private API (for audit.c only) */ -extern int audit_rule_change(int type, __u32 portid, int seq, - void *data, size_t datasz); +extern int audit_rule_change(int type, int seq, void *data, size_t datasz); extern int audit_list_rules_send(struct sk_buff *request_skb, int seq); extern u32 audit_enabled; diff --git a/kernel/audit.c b/kernel/audit.c index eff602c1aa79..b40f3c4727e1 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -250,14 +250,6 @@ static struct sock *audit_get_sk(const struct net *net) return aunet->sk; } -static void audit_set_portid(struct audit_buffer *ab, __u32 portid) -{ - if (ab) { - struct nlmsghdr *nlh = nlmsg_hdr(ab->skb); - nlh->nlmsg_pid = portid; - } -} - void audit_panic(const char *message) { switch (audit_failure) { @@ -816,7 +808,7 @@ int audit_send_list(void *_dest) return 0; } -struct sk_buff *audit_make_reply(__u32 portid, int seq, int type, int done, +struct sk_buff *audit_make_reply(int seq, int type, int done, int multi, const void *payload, int size) { struct sk_buff *skb; @@ -829,7 +821,7 @@ struct sk_buff *audit_make_reply(__u32 portid, int seq, int type, int done, if (!skb) return NULL; - nlh = nlmsg_put(skb, portid, seq, t, size, flags); + nlh = nlmsg_put(skb, 0, seq, t, size, flags); if (!nlh) goto out_kfree_skb; data = nlmsg_data(nlh); @@ -873,7 +865,6 @@ static int audit_send_reply_thread(void *arg) static void audit_send_reply(struct sk_buff *request_skb, int seq, int type, int done, int multi, const void *payload, int size) { - u32 portid = NETLINK_CB(request_skb).portid; struct net *net = sock_net(NETLINK_CB(request_skb).sk); struct sk_buff *skb; struct task_struct *tsk; @@ -883,12 +874,12 @@ static void audit_send_reply(struct sk_buff *request_skb, int seq, int type, int if (!reply) return; - skb = audit_make_reply(portid, seq, type, done, multi, payload, size); + skb = audit_make_reply(seq, type, done, multi, payload, size); if (!skb) goto out; reply->net = get_net(net); - reply->portid = portid; + reply->portid = NETLINK_CB(request_skb).portid; reply->skb = skb; tsk = kthread_run(audit_send_reply_thread, reply, "audit_send_reply"); @@ -1072,7 +1063,7 @@ static int audit_replace(pid_t pid) { struct sk_buff *skb; - skb = audit_make_reply(0, 0, AUDIT_REPLACE, 0, 0, &pid, sizeof(pid)); + skb = audit_make_reply(0, AUDIT_REPLACE, 0, 0, &pid, sizeof(pid)); if (!skb) return -ENOMEM; return auditd_send_unicast_skb(skb); @@ -1242,7 +1233,6 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh) size--; audit_log_n_untrustedstring(ab, data, size); } - audit_set_portid(ab, NETLINK_CB(skb).portid); audit_log_end(ab); } break; @@ -1256,8 +1246,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh) audit_log_end(ab); return -EPERM; } - err = audit_rule_change(msg_type, NETLINK_CB(skb).portid, - seq, data, nlmsg_len(nlh)); + err = audit_rule_change(msg_type, seq, data, nlmsg_len(nlh)); break; case AUDIT_LIST_RULES: err = audit_list_rules_send(skb, seq); diff --git a/kernel/audit.h b/kernel/audit.h index 0d87f8ab8778..18f3c2deeccf 100644 --- a/kernel/audit.h +++ b/kernel/audit.h @@ -237,8 +237,7 @@ extern int audit_uid_comparator(kuid_t left, u32 op, kuid_t right); extern int audit_gid_comparator(kgid_t left, u32 op, kgid_t right); extern int parent_len(const char *path); extern int audit_compare_dname_path(const char *dname, const char *path, int plen); -extern struct sk_buff *audit_make_reply(__u32 portid, int seq, int type, - int done, int multi, +extern struct sk_buff *audit_make_reply(int seq, int type, int done, int multi, const void *payload, int size); extern void audit_panic(const char *message); diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c index 239d11c3122c..0b0aa5854dac 100644 --- a/kernel/auditfilter.c +++ b/kernel/auditfilter.c @@ -1033,7 +1033,7 @@ int audit_del_rule(struct audit_entry *entry) } /* List rules using struct audit_rule_data. */ -static void audit_list_rules(__u32 portid, int seq, struct sk_buff_head *q) +static void audit_list_rules(int seq, struct sk_buff_head *q) { struct sk_buff *skb; struct audit_krule *r; @@ -1048,15 +1048,15 @@ static void audit_list_rules(__u32 portid, int seq, struct sk_buff_head *q) data = audit_krule_to_data(r); if (unlikely(!data)) break; - skb = audit_make_reply(portid, seq, AUDIT_LIST_RULES, - 0, 1, data, + skb = audit_make_reply(seq, AUDIT_LIST_RULES, 0, 1, + data, sizeof(*data) + data->buflen); if (skb) skb_queue_tail(q, skb); kfree(data); } } - skb = audit_make_reply(portid, seq, AUDIT_LIST_RULES, 1, 1, NULL, 0); + skb = audit_make_reply(seq, AUDIT_LIST_RULES, 1, 1, NULL, 0); if (skb) skb_queue_tail(q, skb); } @@ -1085,13 +1085,11 @@ static void audit_log_rule_change(char *action, struct audit_krule *rule, int re /** * audit_rule_change - apply all rules to the specified message type * @type: audit message type - * @portid: target port id for netlink audit messages * @seq: netlink audit message sequence (serial) number * @data: payload data * @datasz: size of payload data */ -int audit_rule_change(int type, __u32 portid, int seq, void *data, - size_t datasz) +int audit_rule_change(int type, int seq, void *data, size_t datasz) { int err = 0; struct audit_entry *entry; @@ -1150,7 +1148,7 @@ int audit_list_rules_send(struct sk_buff *request_skb, int seq) skb_queue_head_init(&dest->q); mutex_lock(&audit_filter_mutex); - audit_list_rules(portid, seq, &dest->q); + audit_list_rules(seq, &dest->q); mutex_unlock(&audit_filter_mutex); tsk = kthread_run(audit_send_list, dest, "audit_send_list"); From b6c7c115c2ce679ac536f0adf0ff518fcd939196 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Tue, 2 May 2017 10:16:05 -0400 Subject: [PATCH 11/14] audit: store the auditd PID as a pid struct instead of pid_t This is arguably the right thing to do, and will make it easier when we start supporting multiple audit daemons in different namespaces. Signed-off-by: Paul Moore --- kernel/audit.c | 84 ++++++++++++++++++++++++++++++++++---------------- kernel/audit.h | 2 +- 2 files changed, 58 insertions(+), 28 deletions(-) diff --git a/kernel/audit.c b/kernel/audit.c index b40f3c4727e1..a2f7803a68d0 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -58,6 +58,7 @@ #include #include #include +#include #include @@ -117,7 +118,7 @@ struct audit_net { * or the included spinlock for writing. */ static struct auditd_connection { - int pid; + struct pid *pid; u32 portid; struct net *net; spinlock_t lock; @@ -220,17 +221,40 @@ struct audit_reply { * Description: * Return 1 if the task is a registered audit daemon, 0 otherwise. */ -int auditd_test_task(const struct task_struct *task) +int auditd_test_task(struct task_struct *task) { int rc; rcu_read_lock(); - rc = (auditd_conn.pid && task->tgid == auditd_conn.pid ? 1 : 0); + rc = (auditd_conn.pid && auditd_conn.pid == task_tgid(task) ? 1 : 0); rcu_read_unlock(); return rc; } +/** + * auditd_pid_vnr - Return the auditd PID relative to the namespace + * @auditd: the auditd connection + * + * Description: + * Returns the PID in relation to the namespace, 0 on failure. This function + * takes the RCU read lock internally, but if the caller needs to protect the + * auditd_connection pointer it should take the RCU read lock as well. + */ +static pid_t auditd_pid_vnr(const struct auditd_connection *auditd) +{ + pid_t pid; + + rcu_read_lock(); + if (!auditd || !auditd->pid) + pid = 0; + else + pid = pid_vnr(auditd->pid); + rcu_read_unlock(); + + return pid; +} + /** * audit_get_sk - Return the audit socket for the given network namespace * @net: the destination network namespace @@ -428,12 +452,17 @@ static int audit_set_failure(u32 state) * This function will obtain and drop network namespace references as * necessary. */ -static void auditd_set(int pid, u32 portid, struct net *net) +static void auditd_set(struct pid *pid, u32 portid, struct net *net) { unsigned long flags; spin_lock_irqsave(&auditd_conn.lock, flags); - auditd_conn.pid = pid; + if (auditd_conn.pid) + put_pid(auditd_conn.pid); + if (pid) + auditd_conn.pid = get_pid(pid); + else + auditd_conn.pid = NULL; auditd_conn.portid = portid; if (auditd_conn.net) put_net(auditd_conn.net); @@ -1059,11 +1088,13 @@ static int audit_set_feature(struct sk_buff *skb) return 0; } -static int audit_replace(pid_t pid) +static int audit_replace(struct pid *pid) { + pid_t pvnr; struct sk_buff *skb; - skb = audit_make_reply(0, AUDIT_REPLACE, 0, 0, &pid, sizeof(pid)); + pvnr = pid_vnr(pid); + skb = audit_make_reply(0, AUDIT_REPLACE, 0, 0, &pvnr, sizeof(pvnr)); if (!skb) return -ENOMEM; return auditd_send_unicast_skb(skb); @@ -1093,9 +1124,9 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh) memset(&s, 0, sizeof(s)); s.enabled = audit_enabled; s.failure = audit_failure; - rcu_read_lock(); - s.pid = auditd_conn.pid; - rcu_read_unlock(); + /* NOTE: use pid_vnr() so the PID is relative to the current + * namespace */ + s.pid = auditd_pid_vnr(&auditd_conn); s.rate_limit = audit_rate_limit; s.backlog_limit = audit_backlog_limit; s.lost = atomic_read(&audit_lost); @@ -1121,36 +1152,36 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh) return err; } if (s.mask & AUDIT_STATUS_PID) { - /* NOTE: we are using task_tgid_vnr() below because - * the s.pid value is relative to the namespace - * of the caller; at present this doesn't matter - * much since you can really only run auditd - * from the initial pid namespace, but something - * to keep in mind if this changes */ - int new_pid = s.pid; + /* NOTE: we are using the vnr PID functions below + * because the s.pid value is relative to the + * namespace of the caller; at present this + * doesn't matter much since you can really only + * run auditd from the initial pid namespace, but + * something to keep in mind if this changes */ + pid_t new_pid = s.pid; pid_t auditd_pid; - pid_t requesting_pid = task_tgid_vnr(current); + struct pid *req_pid = task_tgid(current); + + /* sanity check - PID values must match */ + if (new_pid != pid_vnr(req_pid)) + return -EINVAL; /* test the auditd connection */ - audit_replace(requesting_pid); + audit_replace(req_pid); - rcu_read_lock(); - auditd_pid = auditd_conn.pid; + auditd_pid = auditd_pid_vnr(&auditd_conn); /* only the current auditd can unregister itself */ - if ((!new_pid) && (requesting_pid != auditd_pid)) { - rcu_read_unlock(); + if ((!new_pid) && (new_pid != auditd_pid)) { audit_log_config_change("audit_pid", new_pid, auditd_pid, 0); return -EACCES; } /* replacing a healthy auditd is not allowed */ if (auditd_pid && new_pid) { - rcu_read_unlock(); audit_log_config_change("audit_pid", new_pid, auditd_pid, 0); return -EEXIST; } - rcu_read_unlock(); if (audit_enabled != AUDIT_OFF) audit_log_config_change("audit_pid", new_pid, @@ -1158,8 +1189,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh) if (new_pid) { /* register a new auditd connection */ - auditd_set(new_pid, - NETLINK_CB(skb).portid, + auditd_set(req_pid, NETLINK_CB(skb).portid, sock_net(NETLINK_CB(skb).sk)); /* try to process any backlog */ wake_up_interruptible(&kauditd_wait); diff --git a/kernel/audit.h b/kernel/audit.h index 18f3c2deeccf..4987ea2a4702 100644 --- a/kernel/audit.h +++ b/kernel/audit.h @@ -218,7 +218,7 @@ extern void audit_log_name(struct audit_context *context, struct audit_names *n, const struct path *path, int record_num, int *call_panic); -extern int auditd_test_task(const struct task_struct *task); +extern int auditd_test_task(struct task_struct *task); #define AUDIT_INODE_BUCKETS 32 extern struct list_head audit_inode_hash[AUDIT_INODE_BUCKETS]; From 2115bb250f260089743e26decfb5f271ba71ca37 Mon Sep 17 00:00:00 2001 From: Deepa Dinamani Date: Tue, 2 May 2017 10:16:05 -0400 Subject: [PATCH 12/14] audit: Use timespec64 to represent audit timestamps struct timespec is not y2038 safe. Audit timestamps are recorded in string format into an audit buffer for a given context. These mark the entry timestamps for the syscalls. Use y2038 safe struct timespec64 to represent the times. The log strings can handle this transition as strings can hold upto 1024 characters. Signed-off-by: Deepa Dinamani Reviewed-by: Arnd Bergmann Acked-by: Paul Moore Acked-by: Richard Guy Briggs Signed-off-by: Paul Moore --- include/linux/audit.h | 4 ++-- kernel/audit.c | 10 +++++----- kernel/audit.h | 2 +- kernel/auditsc.c | 6 +++--- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/include/linux/audit.h b/include/linux/audit.h index cc0497c39472..2150bdccfbab 100644 --- a/include/linux/audit.h +++ b/include/linux/audit.h @@ -331,7 +331,7 @@ static inline void audit_ptrace(struct task_struct *t) /* Private API (for audit.c only) */ extern unsigned int audit_serial(void); extern int auditsc_get_stamp(struct audit_context *ctx, - struct timespec *t, unsigned int *serial); + struct timespec64 *t, unsigned int *serial); extern int audit_set_loginuid(kuid_t loginuid); static inline kuid_t audit_get_loginuid(struct task_struct *tsk) @@ -510,7 +510,7 @@ static inline void __audit_seccomp(unsigned long syscall, long signr, int code) static inline void audit_seccomp(unsigned long syscall, long signr, int code) { } static inline int auditsc_get_stamp(struct audit_context *ctx, - struct timespec *t, unsigned int *serial) + struct timespec64 *t, unsigned int *serial) { return 0; } diff --git a/kernel/audit.c b/kernel/audit.c index a2f7803a68d0..41efd2ad1931 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -1638,10 +1638,10 @@ unsigned int audit_serial(void) } static inline void audit_get_stamp(struct audit_context *ctx, - struct timespec *t, unsigned int *serial) + struct timespec64 *t, unsigned int *serial) { if (!ctx || !auditsc_get_stamp(ctx, t, serial)) { - *t = CURRENT_TIME; + ktime_get_real_ts64(t); *serial = audit_serial(); } } @@ -1665,7 +1665,7 @@ struct audit_buffer *audit_log_start(struct audit_context *ctx, gfp_t gfp_mask, int type) { struct audit_buffer *ab; - struct timespec t; + struct timespec64 t; unsigned int uninitialized_var(serial); if (audit_initialized != AUDIT_INITIALIZED) @@ -1718,8 +1718,8 @@ struct audit_buffer *audit_log_start(struct audit_context *ctx, gfp_t gfp_mask, } audit_get_stamp(ab->ctx, &t, &serial); - audit_log_format(ab, "audit(%lu.%03lu:%u): ", - t.tv_sec, t.tv_nsec/1000000, serial); + audit_log_format(ab, "audit(%llu.%03lu:%u): ", + (unsigned long long)t.tv_sec, t.tv_nsec/1000000, serial); return ab; } diff --git a/kernel/audit.h b/kernel/audit.h index 4987ea2a4702..ddfce2ea4891 100644 --- a/kernel/audit.h +++ b/kernel/audit.h @@ -112,7 +112,7 @@ struct audit_context { enum audit_state state, current_state; unsigned int serial; /* serial number for record */ int major; /* syscall number */ - struct timespec ctime; /* time of syscall entry */ + struct timespec64 ctime; /* time of syscall entry */ unsigned long argv[4]; /* syscall arguments */ long return_code;/* syscall return code */ u64 prio; diff --git a/kernel/auditsc.c b/kernel/auditsc.c index 1c2333155893..b2dcbe637b7c 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -1532,7 +1532,7 @@ void __audit_syscall_entry(int major, unsigned long a1, unsigned long a2, return; context->serial = 0; - context->ctime = CURRENT_TIME; + ktime_get_real_ts64(&context->ctime); context->in_syscall = 1; context->current_state = state; context->ppid = 0; @@ -1941,13 +1941,13 @@ EXPORT_SYMBOL_GPL(__audit_inode_child); /** * auditsc_get_stamp - get local copies of audit_context values * @ctx: audit_context for the task - * @t: timespec to store time recorded in the audit_context + * @t: timespec64 to store time recorded in the audit_context * @serial: serial value that is recorded in the audit_context * * Also sets the context as auditable. */ int auditsc_get_stamp(struct audit_context *ctx, - struct timespec *t, unsigned int *serial) + struct timespec64 *t, unsigned int *serial) { if (!ctx->in_syscall) return 0; From 8cc96382d9a7fe1746286670dd5140c3b12638ae Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Tue, 2 May 2017 10:16:05 -0400 Subject: [PATCH 13/14] audit: use kmem_cache to manage the audit_buffer cache The audit subsystem implemented its own buffer cache mechanism which is a bit silly these days when we could use the kmem_cache construct. Some credit is due to Florian Westphal for originally proposing that we remove the audit cache implementation in favor of simple kmalloc()/kfree() calls, but I would rather have a dedicated slab cache to ease debugging and future stats/performance work. Cc: Florian Westphal Reviewed-by: Richard Guy Briggs Signed-off-by: Paul Moore --- kernel/audit.c | 66 +++++++++++++------------------------------------- 1 file changed, 17 insertions(+), 49 deletions(-) diff --git a/kernel/audit.c b/kernel/audit.c index 41efd2ad1931..10bc2bad2adf 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -59,6 +59,7 @@ #include #include #include +#include #include @@ -152,12 +153,7 @@ static atomic_t audit_lost = ATOMIC_INIT(0); /* Hash for inode-based rules */ struct list_head audit_inode_hash[AUDIT_INODE_BUCKETS]; -/* The audit_freelist is a list of pre-allocated audit buffers (if more - * than AUDIT_MAXFREE are in use, the audit buffer is freed instead of - * being placed on the freelist). */ -static DEFINE_SPINLOCK(audit_freelist_lock); -static int audit_freelist_count; -static LIST_HEAD(audit_freelist); +static struct kmem_cache *audit_buffer_cache; /* queue msgs to send via kauditd_task */ static struct sk_buff_head audit_queue; @@ -192,17 +188,12 @@ DEFINE_MUTEX(audit_cmd_mutex); * should be at least that large. */ #define AUDIT_BUFSIZ 1024 -/* AUDIT_MAXFREE is the number of empty audit_buffers we keep on the - * audit_freelist. Doing so eliminates many kmalloc/kfree calls. */ -#define AUDIT_MAXFREE (2*NR_CPUS) - /* The audit_buffer is used when formatting an audit record. The caller * locks briefly to get the record off the freelist or to allocate the * buffer, and locks briefly to send the buffer to the netlink layer or * to place it on a transmit queue. Multiple audit_buffers can be in * use simultaneously. */ struct audit_buffer { - struct list_head list; struct sk_buff *skb; /* formatted skb ready to send */ struct audit_context *ctx; /* NULL or associated context */ gfp_t gfp_mask; @@ -1486,6 +1477,10 @@ static int __init audit_init(void) if (audit_initialized == AUDIT_DISABLED) return 0; + audit_buffer_cache = kmem_cache_create("audit_buffer", + sizeof(struct audit_buffer), + 0, SLAB_PANIC, NULL); + memset(&auditd_conn, 0, sizeof(auditd_conn)); spin_lock_init(&auditd_conn.lock); @@ -1554,60 +1549,33 @@ __setup("audit_backlog_limit=", audit_backlog_limit_set); static void audit_buffer_free(struct audit_buffer *ab) { - unsigned long flags; - if (!ab) return; kfree_skb(ab->skb); - spin_lock_irqsave(&audit_freelist_lock, flags); - if (audit_freelist_count > AUDIT_MAXFREE) - kfree(ab); - else { - audit_freelist_count++; - list_add(&ab->list, &audit_freelist); - } - spin_unlock_irqrestore(&audit_freelist_lock, flags); + kmem_cache_free(audit_buffer_cache, ab); } -static struct audit_buffer * audit_buffer_alloc(struct audit_context *ctx, - gfp_t gfp_mask, int type) +static struct audit_buffer *audit_buffer_alloc(struct audit_context *ctx, + gfp_t gfp_mask, int type) { - unsigned long flags; - struct audit_buffer *ab = NULL; - struct nlmsghdr *nlh; + struct audit_buffer *ab; - spin_lock_irqsave(&audit_freelist_lock, flags); - if (!list_empty(&audit_freelist)) { - ab = list_entry(audit_freelist.next, - struct audit_buffer, list); - list_del(&ab->list); - --audit_freelist_count; - } - spin_unlock_irqrestore(&audit_freelist_lock, flags); - - if (!ab) { - ab = kmalloc(sizeof(*ab), gfp_mask); - if (!ab) - goto err; - } - - ab->ctx = ctx; - ab->gfp_mask = gfp_mask; + ab = kmem_cache_alloc(audit_buffer_cache, gfp_mask); + if (!ab) + return NULL; ab->skb = nlmsg_new(AUDIT_BUFSIZ, gfp_mask); if (!ab->skb) goto err; + if (!nlmsg_put(ab->skb, 0, 0, type, 0, 0)) + goto err; - nlh = nlmsg_put(ab->skb, 0, 0, type, 0, 0); - if (!nlh) - goto out_kfree_skb; + ab->ctx = ctx; + ab->gfp_mask = gfp_mask; return ab; -out_kfree_skb: - kfree_skb(ab->skb); - ab->skb = NULL; err: audit_buffer_free(ab); return NULL; From 48d0e023af9799cd7220335baf8e3ba61eeafbeb Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Tue, 2 May 2017 10:16:05 -0400 Subject: [PATCH 14/14] audit: fix the RCU locking for the auditd_connection structure Cong Wang correctly pointed out that the RCU read locking of the auditd_connection struct was wrong, this patch correct this by adopting a more traditional, and correct RCU locking model. This patch is heavily based on an earlier prototype by Cong Wang. Cc: # 4.11.x- Reported-by: Cong Wang Signed-off-by: Cong Wang Signed-off-by: Paul Moore --- kernel/audit.c | 157 +++++++++++++++++++++++++++++++------------------ 1 file changed, 100 insertions(+), 57 deletions(-) diff --git a/kernel/audit.c b/kernel/audit.c index 10bc2bad2adf..a7c6a50477aa 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -112,18 +112,19 @@ struct audit_net { * @pid: auditd PID * @portid: netlink portid * @net: the associated network namespace - * @lock: spinlock to protect write access + * @rcu: RCU head * * Description: * This struct is RCU protected; you must either hold the RCU lock for reading - * or the included spinlock for writing. + * or the associated spinlock for writing. */ static struct auditd_connection { struct pid *pid; u32 portid; struct net *net; - spinlock_t lock; -} auditd_conn; + struct rcu_head rcu; +} *auditd_conn = NULL; +static DEFINE_SPINLOCK(auditd_conn_lock); /* If audit_rate_limit is non-zero, limit the rate of sending audit records * to that number per second. This prevents DoS attacks, but results in @@ -215,9 +216,11 @@ struct audit_reply { int auditd_test_task(struct task_struct *task) { int rc; + struct auditd_connection *ac; rcu_read_lock(); - rc = (auditd_conn.pid && auditd_conn.pid == task_tgid(task) ? 1 : 0); + ac = rcu_dereference(auditd_conn); + rc = (ac && ac->pid == task_tgid(task) ? 1 : 0); rcu_read_unlock(); return rc; @@ -225,22 +228,21 @@ int auditd_test_task(struct task_struct *task) /** * auditd_pid_vnr - Return the auditd PID relative to the namespace - * @auditd: the auditd connection * * Description: - * Returns the PID in relation to the namespace, 0 on failure. This function - * takes the RCU read lock internally, but if the caller needs to protect the - * auditd_connection pointer it should take the RCU read lock as well. + * Returns the PID in relation to the namespace, 0 on failure. */ -static pid_t auditd_pid_vnr(const struct auditd_connection *auditd) +static pid_t auditd_pid_vnr(void) { pid_t pid; + const struct auditd_connection *ac; rcu_read_lock(); - if (!auditd || !auditd->pid) + ac = rcu_dereference(auditd_conn); + if (!ac || !ac->pid) pid = 0; else - pid = pid_vnr(auditd->pid); + pid = pid_vnr(ac->pid); rcu_read_unlock(); return pid; @@ -433,6 +435,24 @@ static int audit_set_failure(u32 state) return audit_do_config_change("audit_failure", &audit_failure, state); } +/** + * auditd_conn_free - RCU helper to release an auditd connection struct + * @rcu: RCU head + * + * Description: + * Drop any references inside the auditd connection tracking struct and free + * the memory. + */ + static void auditd_conn_free(struct rcu_head *rcu) + { + struct auditd_connection *ac; + + ac = container_of(rcu, struct auditd_connection, rcu); + put_pid(ac->pid); + put_net(ac->net); + kfree(ac); + } + /** * auditd_set - Set/Reset the auditd connection state * @pid: auditd PID @@ -441,27 +461,33 @@ static int audit_set_failure(u32 state) * * Description: * This function will obtain and drop network namespace references as - * necessary. + * necessary. Returns zero on success, negative values on failure. */ -static void auditd_set(struct pid *pid, u32 portid, struct net *net) +static int auditd_set(struct pid *pid, u32 portid, struct net *net) { unsigned long flags; + struct auditd_connection *ac_old, *ac_new; - spin_lock_irqsave(&auditd_conn.lock, flags); - if (auditd_conn.pid) - put_pid(auditd_conn.pid); - if (pid) - auditd_conn.pid = get_pid(pid); - else - auditd_conn.pid = NULL; - auditd_conn.portid = portid; - if (auditd_conn.net) - put_net(auditd_conn.net); - if (net) - auditd_conn.net = get_net(net); - else - auditd_conn.net = NULL; - spin_unlock_irqrestore(&auditd_conn.lock, flags); + if (!pid || !net) + return -EINVAL; + + ac_new = kzalloc(sizeof(*ac_new), GFP_KERNEL); + if (!ac_new) + return -ENOMEM; + ac_new->pid = get_pid(pid); + ac_new->portid = portid; + ac_new->net = get_net(net); + + spin_lock_irqsave(&auditd_conn_lock, flags); + ac_old = rcu_dereference_protected(auditd_conn, + lockdep_is_held(&auditd_conn_lock)); + rcu_assign_pointer(auditd_conn, ac_new); + spin_unlock_irqrestore(&auditd_conn_lock, flags); + + if (ac_old) + call_rcu(&ac_old->rcu, auditd_conn_free); + + return 0; } /** @@ -556,13 +582,19 @@ static void kauditd_retry_skb(struct sk_buff *skb) */ static void auditd_reset(void) { + unsigned long flags; struct sk_buff *skb; + struct auditd_connection *ac_old; /* if it isn't already broken, break the connection */ - rcu_read_lock(); - if (auditd_conn.pid) - auditd_set(0, 0, NULL); - rcu_read_unlock(); + spin_lock_irqsave(&auditd_conn_lock, flags); + ac_old = rcu_dereference_protected(auditd_conn, + lockdep_is_held(&auditd_conn_lock)); + rcu_assign_pointer(auditd_conn, NULL); + spin_unlock_irqrestore(&auditd_conn_lock, flags); + + if (ac_old) + call_rcu(&ac_old->rcu, auditd_conn_free); /* flush all of the main and retry queues to the hold queue */ while ((skb = skb_dequeue(&audit_retry_queue))) @@ -588,6 +620,7 @@ static int auditd_send_unicast_skb(struct sk_buff *skb) u32 portid; struct net *net; struct sock *sk; + struct auditd_connection *ac; /* NOTE: we can't call netlink_unicast while in the RCU section so * take a reference to the network namespace and grab local @@ -597,15 +630,15 @@ static int auditd_send_unicast_skb(struct sk_buff *skb) * section netlink_unicast() should safely return an error */ rcu_read_lock(); - if (!auditd_conn.pid) { + ac = rcu_dereference(auditd_conn); + if (!ac) { rcu_read_unlock(); rc = -ECONNREFUSED; goto err; } - net = auditd_conn.net; - get_net(net); + net = get_net(ac->net); sk = audit_get_sk(net); - portid = auditd_conn.portid; + portid = ac->portid; rcu_read_unlock(); rc = netlink_unicast(sk, skb, portid, 0); @@ -740,6 +773,7 @@ static int kauditd_thread(void *dummy) u32 portid = 0; struct net *net = NULL; struct sock *sk = NULL; + struct auditd_connection *ac; #define UNICAST_RETRIES 5 @@ -747,14 +781,14 @@ static int kauditd_thread(void *dummy) while (!kthread_should_stop()) { /* NOTE: see the lock comments in auditd_send_unicast_skb() */ rcu_read_lock(); - if (!auditd_conn.pid) { + ac = rcu_dereference(auditd_conn); + if (!ac) { rcu_read_unlock(); goto main_queue; } - net = auditd_conn.net; - get_net(net); + net = get_net(ac->net); sk = audit_get_sk(net); - portid = auditd_conn.portid; + portid = ac->portid; rcu_read_unlock(); /* attempt to flush the hold queue */ @@ -1117,7 +1151,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh) s.failure = audit_failure; /* NOTE: use pid_vnr() so the PID is relative to the current * namespace */ - s.pid = auditd_pid_vnr(&auditd_conn); + s.pid = auditd_pid_vnr(); s.rate_limit = audit_rate_limit; s.backlog_limit = audit_backlog_limit; s.lost = atomic_read(&audit_lost); @@ -1160,7 +1194,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh) /* test the auditd connection */ audit_replace(req_pid); - auditd_pid = auditd_pid_vnr(&auditd_conn); + auditd_pid = auditd_pid_vnr(); /* only the current auditd can unregister itself */ if ((!new_pid) && (new_pid != auditd_pid)) { audit_log_config_change("audit_pid", new_pid, @@ -1174,19 +1208,30 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh) return -EEXIST; } - if (audit_enabled != AUDIT_OFF) - audit_log_config_change("audit_pid", new_pid, - auditd_pid, 1); - if (new_pid) { /* register a new auditd connection */ - auditd_set(req_pid, NETLINK_CB(skb).portid, - sock_net(NETLINK_CB(skb).sk)); + err = auditd_set(req_pid, + NETLINK_CB(skb).portid, + sock_net(NETLINK_CB(skb).sk)); + if (audit_enabled != AUDIT_OFF) + audit_log_config_change("audit_pid", + new_pid, + auditd_pid, + err ? 0 : 1); + if (err) + return err; + /* try to process any backlog */ wake_up_interruptible(&kauditd_wait); - } else + } else { + if (audit_enabled != AUDIT_OFF) + audit_log_config_change("audit_pid", + new_pid, + auditd_pid, 1); + /* unregister the auditd connection */ auditd_reset(); + } } if (s.mask & AUDIT_STATUS_RATE_LIMIT) { err = audit_set_rate_limit(s.rate_limit); @@ -1454,10 +1499,11 @@ static void __net_exit audit_net_exit(struct net *net) { struct audit_net *aunet = net_generic(net, audit_net_id); - rcu_read_lock(); - if (net == auditd_conn.net) - auditd_reset(); - rcu_read_unlock(); + /* NOTE: you would think that we would want to check the auditd + * connection and potentially reset it here if it lives in this + * namespace, but since the auditd connection tracking struct holds a + * reference to this namespace (see auditd_set()) we are only ever + * going to get here after that connection has been released */ netlink_kernel_release(aunet->sk); } @@ -1481,9 +1527,6 @@ static int __init audit_init(void) sizeof(struct audit_buffer), 0, SLAB_PANIC, NULL); - memset(&auditd_conn, 0, sizeof(auditd_conn)); - spin_lock_init(&auditd_conn.lock); - skb_queue_head_init(&audit_queue); skb_queue_head_init(&audit_retry_queue); skb_queue_head_init(&audit_hold_queue);