From a48b284b403a4a073d8beb72d2bb33e54df67fb6 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Mon, 20 Apr 2020 10:09:29 -0400 Subject: [PATCH 1/8] audit: fix a net reference leak in audit_send_reply() If audit_send_reply() fails when trying to create a new thread to send the reply it also fails to cleanup properly, leaking a reference to a net structure. This patch fixes the error path and makes a handful of other cleanups that came up while fixing the code. Reported-by: teroincn@gmail.com Reviewed-by: Richard Guy Briggs Signed-off-by: Paul Moore --- kernel/audit.c | 50 +++++++++++++++++++++++++++++--------------------- 1 file changed, 29 insertions(+), 21 deletions(-) diff --git a/kernel/audit.c b/kernel/audit.c index b69c8b460341f..66b81358b64f5 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -924,19 +924,30 @@ struct sk_buff *audit_make_reply(int seq, int type, int done, return NULL; } +static void audit_free_reply(struct audit_reply *reply) +{ + if (!reply) + return; + + if (reply->skb) + kfree_skb(reply->skb); + if (reply->net) + put_net(reply->net); + kfree(reply); +} + static int audit_send_reply_thread(void *arg) { struct audit_reply *reply = (struct audit_reply *)arg; - struct sock *sk = audit_get_sk(reply->net); audit_ctl_lock(); audit_ctl_unlock(); /* Ignore failure. It'll only happen if the sender goes away, because our timeout is set to infinite. */ - netlink_unicast(sk, reply->skb, reply->portid, 0); - put_net(reply->net); - kfree(reply); + netlink_unicast(audit_get_sk(reply->net), reply->skb, reply->portid, 0); + reply->skb = NULL; + audit_free_reply(reply); return 0; } @@ -950,35 +961,32 @@ static int audit_send_reply_thread(void *arg) * @payload: payload data * @size: payload size * - * Allocates an skb, builds the netlink message, and sends it to the port id. - * No failure notifications. + * Allocates a skb, builds the netlink message, and sends it to the port id. */ static void audit_send_reply(struct sk_buff *request_skb, int seq, int type, int done, int multi, const void *payload, int size) { - struct net *net = sock_net(NETLINK_CB(request_skb).sk); - struct sk_buff *skb; struct task_struct *tsk; - struct audit_reply *reply = kmalloc(sizeof(struct audit_reply), - GFP_KERNEL); + struct audit_reply *reply; + reply = kzalloc(sizeof(*reply), GFP_KERNEL); if (!reply) return; - skb = audit_make_reply(seq, type, done, multi, payload, size); - if (!skb) - goto out; - - reply->net = get_net(net); + reply->skb = audit_make_reply(seq, type, done, multi, payload, size); + if (!reply->skb) + goto err; + reply->net = get_net(sock_net(NETLINK_CB(request_skb).sk)); reply->portid = NETLINK_CB(request_skb).portid; - reply->skb = skb; tsk = kthread_run(audit_send_reply_thread, reply, "audit_send_reply"); - if (!IS_ERR(tsk)) - return; - kfree_skb(skb); -out: - kfree(reply); + if (IS_ERR(tsk)) + goto err; + + return; + +err: + audit_free_reply(reply); } /* From 3054d06719079388a543de6adb812638675ad8f5 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Tue, 21 Apr 2020 09:10:56 -0400 Subject: [PATCH 2/8] audit: fix a net reference leak in audit_list_rules_send() If audit_list_rules_send() fails when trying to create a new thread to send the rules it also fails to cleanup properly, leaking a reference to a net structure. This patch fixes the error patch and renames audit_send_list() to audit_send_list_thread() to better match its cousin, audit_send_reply_thread(). Reported-by: teroincn@gmail.com Reviewed-by: Richard Guy Briggs Signed-off-by: Paul Moore --- kernel/audit.c | 2 +- kernel/audit.h | 2 +- kernel/auditfilter.c | 16 +++++++--------- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/kernel/audit.c b/kernel/audit.c index 66b81358b64f5..622c30246d197 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -880,7 +880,7 @@ static int kauditd_thread(void *dummy) return 0; } -int audit_send_list(void *_dest) +int audit_send_list_thread(void *_dest) { struct audit_netlink_list *dest = _dest; struct sk_buff *skb; diff --git a/kernel/audit.h b/kernel/audit.h index 2eed4d2316246..f0233dc40b173 100644 --- a/kernel/audit.h +++ b/kernel/audit.h @@ -229,7 +229,7 @@ struct audit_netlink_list { struct sk_buff_head q; }; -int audit_send_list(void *_dest); +int audit_send_list_thread(void *_dest); extern int selinux_audit_rule_update(void); diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c index 026e34da4ace9..a10e2997aa6c8 100644 --- a/kernel/auditfilter.c +++ b/kernel/auditfilter.c @@ -1161,11 +1161,8 @@ int audit_rule_change(int type, int seq, void *data, size_t datasz) */ int audit_list_rules_send(struct sk_buff *request_skb, int seq) { - u32 portid = NETLINK_CB(request_skb).portid; - struct net *net = sock_net(NETLINK_CB(request_skb).sk); struct task_struct *tsk; struct audit_netlink_list *dest; - int err = 0; /* We can't just spew out the rules here because we might fill * the available socket buffer space and deadlock waiting for @@ -1173,25 +1170,26 @@ int audit_list_rules_send(struct sk_buff *request_skb, int seq) * happen if we're actually running in the context of auditctl * trying to _send_ the stuff */ - dest = kmalloc(sizeof(struct audit_netlink_list), GFP_KERNEL); + dest = kmalloc(sizeof(*dest), GFP_KERNEL); if (!dest) return -ENOMEM; - dest->net = get_net(net); - dest->portid = portid; + dest->net = get_net(sock_net(NETLINK_CB(request_skb).sk)); + dest->portid = NETLINK_CB(request_skb).portid; skb_queue_head_init(&dest->q); mutex_lock(&audit_filter_mutex); audit_list_rules(seq, &dest->q); mutex_unlock(&audit_filter_mutex); - tsk = kthread_run(audit_send_list, dest, "audit_send_list"); + tsk = kthread_run(audit_send_list_thread, dest, "audit_send_list"); if (IS_ERR(tsk)) { skb_queue_purge(&dest->q); + put_net(dest->net); kfree(dest); - err = PTR_ERR(tsk); + return PTR_ERR(tsk); } - return err; + return 0; } int audit_comparator(u32 left, u32 op, u32 right) From 9d2161bed4e39ef7a5e5f18f69c4a57d001051b9 Mon Sep 17 00:00:00 2001 From: Richard Guy Briggs Date: Wed, 22 Apr 2020 17:37:04 -0400 Subject: [PATCH 3/8] audit: log audit netlink multicast bind and unbind Log information about programs connecting to and disconnecting from the audit netlink multicast socket. This is needed so that during investigations a security officer can tell who or what had access to the audit trail. This helps to meet the FAU_SAR.2 requirement for Common Criteria. Here is the systemd startup event: type=PROCTITLE msg=audit(2020-04-22 10:10:21.787:10) : proctitle=/init type=SYSCALL msg=audit(2020-04-22 10:10:21.787:10) : arch=x86_64 syscall=bind success=yes exit=0 a0=0x19 a1=0x555f4aac7e90 a2=0xc a3=0x7ffcb792ff44 items=0 ppid=0 pid=1 auid=unset uid=root gid=root euid=root suid=root fsuid=root egid=root sgid=root fsgid=root tty=(none) ses=unset comm=systemd exe=/usr/lib/systemd/systemd subj=kernel key=(null) type=UNKNOWN[1335] msg=audit(2020-04-22 10:10:21.787:10) : pid=1 uid=root auid=unset tty=(none) ses=unset subj=kernel comm=systemd exe=/usr/lib/systemd/systemd nl-mcgrp=1 op=connect res=yes And events from the test suite that just uses close(): type=PROCTITLE msg=audit(2020-04-22 11:47:08.501:442) : proctitle=/usr/bin/perl -w amcast_joinpart/test type=SYSCALL msg=audit(2020-04-22 11:47:08.501:442) : arch=x86_64 syscall=bind success=yes exit=0 a0=0x7 a1=0x563004378760 a2=0xc a3=0x0 items=0 ppid=815 pid=818 auid=root uid=root gid=root euid=root suid=root fsuid=root egid=root sgid=root fsgid=root tty=ttyS0 ses=1 comm=perl exe=/usr/bin/perl subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key=(null) type=UNKNOWN[1335] msg=audit(2020-04-22 11:47:08.501:442) : pid=818 uid=root auid=root tty=ttyS0 ses=1 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 comm=perl exe=/usr/bin/perl nl-mcgrp=1 op=connect res=yes type=UNKNOWN[1335] msg=audit(2020-04-22 11:47:08.501:443) : pid=818 uid=root auid=root tty=ttyS0 ses=1 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 comm=perl exe=/usr/bin/perl nl-mcgrp=1 op=disconnect res=yes And the events from the test suite using setsockopt with NETLINK_DROP_MEMBERSHIP: type=PROCTITLE msg=audit(2020-04-22 11:39:53.291:439) : proctitle=/usr/bin/perl -w amcast_joinpart/test type=SYSCALL msg=audit(2020-04-22 11:39:53.291:439) : arch=x86_64 syscall=bind success=yes exit=0 a0=0x7 a1=0x5560877c2d20 a2=0xc a3=0x0 items=0 ppid=772 pid=775 auid=root uid=root gid=root euid=root suid=root fsuid=root egid=root sgid=root fsgid=root tty=ttyS0 ses=1 comm=perl exe=/usr/bin/perl subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key=(null) type=UNKNOWN[1335] msg=audit(2020-04-22 11:39:53.291:439) : pid=775 uid=root auid=root tty=ttyS0 ses=1 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 comm=perl exe=/usr/bin/perl nl-mcgrp=1 op=connect res=yes type=PROCTITLE msg=audit(2020-04-22 11:39:53.292:440) : proctitle=/usr/bin/perl -w amcast_joinpart/test type=SYSCALL msg=audit(2020-04-22 11:39:53.292:440) : arch=x86_64 syscall=setsockopt success=yes exit=0 a0=0x7 a1=SOL_NETLINK a2=0x2 a3=0x7ffc8366f000 items=0 ppid=772 pid=775 auid=root uid=root gid=root euid=root suid=root fsuid=root egid=root sgid=root fsgid=root tty=ttyS0 ses=1 comm=perl exe=/usr/bin/perl subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key=(null) type=UNKNOWN[1335] msg=audit(2020-04-22 11:39:53.292:440) : pid=775 uid=root auid=root tty=ttyS0 ses=1 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 comm=perl exe=/usr/bin/perl nl-mcgrp=1 op=disconnect res=yes Please see the upstream issue tracker at https://github.com/linux-audit/audit-kernel/issues/28 With the feature description at https://github.com/linux-audit/audit-kernel/wiki/RFE-Audit-Multicast-Socket-Join-Part The testsuite support is at https://github.com/rgbriggs/audit-testsuite/compare/ghak28-mcast-part-join https://github.com/linux-audit/audit-testsuite/pull/93 And the userspace support patch is at https://github.com/linux-audit/audit-userspace/pull/114 Signed-off-by: Richard Guy Briggs Signed-off-by: Paul Moore --- include/uapi/linux/audit.h | 1 + kernel/audit.c | 48 ++++++++++++++++++++++++++++++++++---- 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h index a534d71e689ac..9b6a973f4cc30 100644 --- a/include/uapi/linux/audit.h +++ b/include/uapi/linux/audit.h @@ -117,6 +117,7 @@ #define AUDIT_TIME_INJOFFSET 1332 /* Timekeeping offset injected */ #define AUDIT_TIME_ADJNTPVAL 1333 /* NTP value adjustment */ #define AUDIT_BPF 1334 /* BPF subsystem */ +#define AUDIT_EVENT_LISTENER 1335 /* Task joined multicast read socket */ #define AUDIT_AVC 1400 /* SE Linux avc denial or grant */ #define AUDIT_SELINUX_ERR 1401 /* Internal SE Linux Errors */ diff --git a/kernel/audit.c b/kernel/audit.c index 622c30246d197..e33460e01b3b7 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -1530,20 +1530,60 @@ static void audit_receive(struct sk_buff *skb) audit_ctl_unlock(); } +/* Log information about who is connecting to the audit multicast socket */ +static void audit_log_multicast(int group, const char *op, int err) +{ + const struct cred *cred; + struct tty_struct *tty; + char comm[sizeof(current->comm)]; + struct audit_buffer *ab; + + if (!audit_enabled) + return; + + ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_EVENT_LISTENER); + if (!ab) + return; + + cred = current_cred(); + tty = audit_get_tty(); + audit_log_format(ab, "pid=%u uid=%u auid=%u tty=%s ses=%u", + task_pid_nr(current), + from_kuid(&init_user_ns, cred->uid), + from_kuid(&init_user_ns, audit_get_loginuid(current)), + tty ? tty_name(tty) : "(none)", + audit_get_sessionid(current)); + audit_put_tty(tty); + audit_log_task_context(ab); /* subj= */ + audit_log_format(ab, " comm="); + audit_log_untrustedstring(ab, get_task_comm(comm, current)); + audit_log_d_path_exe(ab, current->mm); /* exe= */ + audit_log_format(ab, " nl-mcgrp=%d op=%s res=%d", group, op, !err); + audit_log_end(ab); +} + /* Run custom bind function on netlink socket group connect or bind requests. */ -static int audit_bind(struct net *net, int group) +static int audit_multicast_bind(struct net *net, int group) { + int err = 0; + if (!capable(CAP_AUDIT_READ)) - return -EPERM; + err = -EPERM; + audit_log_multicast(group, "connect", err); + return err; +} - return 0; +static void audit_multicast_unbind(struct net *net, int group) +{ + audit_log_multicast(group, "disconnect", 0); } static int __net_init audit_net_init(struct net *net) { struct netlink_kernel_cfg cfg = { .input = audit_receive, - .bind = audit_bind, + .bind = audit_multicast_bind, + .unbind = audit_multicast_unbind, .flags = NL_CFG_F_NONROOT_RECV, .groups = AUDIT_NLGRP_MAX, }; From c4dad0aab3fca0c1f0baa4cc84b6ec91b7ebf426 Mon Sep 17 00:00:00 2001 From: Richard Guy Briggs Date: Wed, 22 Apr 2020 17:39:28 -0400 Subject: [PATCH 4/8] audit: tidy and extend netfilter_cfg x_tables NETFILTER_CFG record generation was inconsistent for x_tables and ebtables configuration changes. The call was needlessly messy and there were supporting records missing at times while they were produced when not requested. Simplify the logging call into a new audit_log_nfcfg call. Honour the audit_enabled setting while more consistently recording information including supporting records by tidying up dummy checks. Add an op= field that indicates the operation being performed (register or replace). Here is the enhanced sample record: type=NETFILTER_CFG msg=audit(1580905834.919:82970): table=filter family=2 entries=83 op=replace Generate audit NETFILTER_CFG records on ebtables table registration. Previously this was being done for x_tables registration and replacement operations and ebtables table replacement only. See: https://github.com/linux-audit/audit-kernel/issues/25 See: https://github.com/linux-audit/audit-kernel/issues/35 See: https://github.com/linux-audit/audit-kernel/issues/43 Signed-off-by: Richard Guy Briggs Signed-off-by: Paul Moore --- include/linux/audit.h | 21 +++++++++++++++++++++ kernel/auditsc.c | 24 ++++++++++++++++++++++++ net/bridge/netfilter/ebtables.c | 12 ++++-------- net/netfilter/x_tables.c | 12 +++--------- 4 files changed, 52 insertions(+), 17 deletions(-) diff --git a/include/linux/audit.h b/include/linux/audit.h index f9ceae57ca8de..5c7f07a9776d2 100644 --- a/include/linux/audit.h +++ b/include/linux/audit.h @@ -94,6 +94,11 @@ struct audit_ntp_data { struct audit_ntp_data {}; #endif +enum audit_nfcfgop { + AUDIT_XT_OP_REGISTER, + AUDIT_XT_OP_REPLACE, +}; + extern int is_audit_feature_set(int which); extern int __init audit_register_class(int class, unsigned *list); @@ -379,6 +384,8 @@ extern void __audit_log_kern_module(char *name); extern void __audit_fanotify(unsigned int response); extern void __audit_tk_injoffset(struct timespec64 offset); extern void __audit_ntp_log(const struct audit_ntp_data *ad); +extern void __audit_log_nfcfg(const char *name, u8 af, unsigned int nentries, + enum audit_nfcfgop op); static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp) { @@ -514,6 +521,14 @@ static inline void audit_ntp_log(const struct audit_ntp_data *ad) __audit_ntp_log(ad); } +static inline void audit_log_nfcfg(const char *name, u8 af, + unsigned int nentries, + enum audit_nfcfgop op) +{ + if (audit_enabled) + __audit_log_nfcfg(name, af, nentries, op); +} + extern int audit_n_rules; extern int audit_signals; #else /* CONFIG_AUDITSYSCALL */ @@ -646,6 +661,12 @@ static inline void audit_ntp_log(const struct audit_ntp_data *ad) static inline void audit_ptrace(struct task_struct *t) { } + +static inline void audit_log_nfcfg(const char *name, u8 af, + unsigned int nentries, + enum audit_nfcfgop op) +{ } + #define audit_n_rules 0 #define audit_signals 0 #endif /* CONFIG_AUDITSYSCALL */ diff --git a/kernel/auditsc.c b/kernel/auditsc.c index 814406a35db16..705beac0ce29b 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -130,6 +130,16 @@ struct audit_tree_refs { struct audit_chunk *c[31]; }; +struct audit_nfcfgop_tab { + enum audit_nfcfgop op; + const char *s; +}; + +const struct audit_nfcfgop_tab audit_nfcfgs[] = { + { AUDIT_XT_OP_REGISTER, "register" }, + { AUDIT_XT_OP_REPLACE, "replace" }, +}; + static int audit_match_perm(struct audit_context *ctx, int mask) { unsigned n; @@ -2542,6 +2552,20 @@ void __audit_ntp_log(const struct audit_ntp_data *ad) audit_log_ntp_val(ad, "adjust", AUDIT_NTP_ADJUST); } +void __audit_log_nfcfg(const char *name, u8 af, unsigned int nentries, + enum audit_nfcfgop op) +{ + struct audit_buffer *ab; + + ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_NETFILTER_CFG); + if (!ab) + return; + audit_log_format(ab, "table=%s family=%u entries=%u op=%s", + name, af, nentries, audit_nfcfgs[op].s); + audit_log_end(ab); +} +EXPORT_SYMBOL_GPL(__audit_log_nfcfg); + static void audit_log_task(struct audit_buffer *ab) { kuid_t auid, uid; diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c index 78db58c7aec2e..0a148e68b6e15 100644 --- a/net/bridge/netfilter/ebtables.c +++ b/net/bridge/netfilter/ebtables.c @@ -1046,14 +1046,8 @@ static int do_replace_finish(struct net *net, struct ebt_replace *repl, vfree(table); vfree(counterstmp); -#ifdef CONFIG_AUDIT - if (audit_enabled) { - audit_log(audit_context(), GFP_KERNEL, - AUDIT_NETFILTER_CFG, - "table=%s family=%u entries=%u", - repl->name, AF_BRIDGE, repl->nentries); - } -#endif + audit_log_nfcfg(repl->name, AF_BRIDGE, repl->nentries, + AUDIT_XT_OP_REPLACE); return ret; free_unlock: @@ -1223,6 +1217,8 @@ int ebt_register_table(struct net *net, const struct ebt_table *input_table, *res = NULL; } + audit_log_nfcfg(repl->name, AF_BRIDGE, repl->nentries, + AUDIT_XT_OP_REGISTER); return ret; free_unlock: mutex_unlock(&ebt_mutex); diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c index cd2b034eef59a..8f8c5dbf603df 100644 --- a/net/netfilter/x_tables.c +++ b/net/netfilter/x_tables.c @@ -1408,15 +1408,9 @@ xt_replace_table(struct xt_table *table, } } -#ifdef CONFIG_AUDIT - if (audit_enabled) { - audit_log(audit_context(), GFP_KERNEL, - AUDIT_NETFILTER_CFG, - "table=%s family=%u entries=%u", - table->name, table->af, private->number); - } -#endif - + audit_log_nfcfg(table->name, table->af, private->number, + !private->number ? AUDIT_XT_OP_REGISTER : + AUDIT_XT_OP_REPLACE); return private; } EXPORT_SYMBOL_GPL(xt_replace_table); From a45d88530b2552ad5ea0da18861600b4ecc9d0c7 Mon Sep 17 00:00:00 2001 From: Richard Guy Briggs Date: Wed, 22 Apr 2020 17:39:29 -0400 Subject: [PATCH 5/8] netfilter: add audit table unregister actions Audit the action of unregistering ebtables and x_tables. See: https://github.com/linux-audit/audit-kernel/issues/44 Signed-off-by: Richard Guy Briggs Signed-off-by: Paul Moore --- include/linux/audit.h | 1 + kernel/auditsc.c | 5 +++-- net/bridge/netfilter/ebtables.c | 2 ++ net/netfilter/x_tables.c | 2 ++ 4 files changed, 8 insertions(+), 2 deletions(-) diff --git a/include/linux/audit.h b/include/linux/audit.h index 5c7f07a9776d2..1a23515082113 100644 --- a/include/linux/audit.h +++ b/include/linux/audit.h @@ -97,6 +97,7 @@ struct audit_ntp_data {}; enum audit_nfcfgop { AUDIT_XT_OP_REGISTER, AUDIT_XT_OP_REPLACE, + AUDIT_XT_OP_UNREGISTER, }; extern int is_audit_feature_set(int which); diff --git a/kernel/auditsc.c b/kernel/auditsc.c index 705beac0ce29b..d281c18d1771a 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -136,8 +136,9 @@ struct audit_nfcfgop_tab { }; const struct audit_nfcfgop_tab audit_nfcfgs[] = { - { AUDIT_XT_OP_REGISTER, "register" }, - { AUDIT_XT_OP_REPLACE, "replace" }, + { AUDIT_XT_OP_REGISTER, "register" }, + { AUDIT_XT_OP_REPLACE, "replace" }, + { AUDIT_XT_OP_UNREGISTER, "unregister" }, }; static int audit_match_perm(struct audit_context *ctx, int mask) diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c index 0a148e68b6e15..4778db5601b0d 100644 --- a/net/bridge/netfilter/ebtables.c +++ b/net/bridge/netfilter/ebtables.c @@ -1124,6 +1124,8 @@ static void __ebt_unregister_table(struct net *net, struct ebt_table *table) mutex_lock(&ebt_mutex); list_del(&table->list); mutex_unlock(&ebt_mutex); + audit_log_nfcfg(table->name, AF_BRIDGE, table->private->nentries, + AUDIT_XT_OP_UNREGISTER); EBT_ENTRY_ITERATE(table->private->entries, table->private->entries_size, ebt_cleanup_entry, net, NULL); if (table->private->nentries) diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c index 8f8c5dbf603df..99a468be4a59f 100644 --- a/net/netfilter/x_tables.c +++ b/net/netfilter/x_tables.c @@ -1472,6 +1472,8 @@ void *xt_unregister_table(struct xt_table *table) private = table->private; list_del(&table->list); mutex_unlock(&xt[table->af].mutex); + audit_log_nfcfg(table->name, table->af, private->number, + AUDIT_XT_OP_UNREGISTER); kfree(table); return private; From db9ff6ecf6efa6ffc45bfd5dc8d5708cfe5e89cb Mon Sep 17 00:00:00 2001 From: Zheng Bin Date: Wed, 29 Apr 2020 17:26:48 +0800 Subject: [PATCH 6/8] audit: make symbol 'audit_nfcfgs' static Fix sparse warnings: kernel/auditsc.c:138:32: warning: symbol 'audit_nfcfgs' was not declared. Should it be static? Reported-by: Hulk Robot Signed-off-by: Zheng Bin Signed-off-by: Paul Moore --- kernel/auditsc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/auditsc.c b/kernel/auditsc.c index d281c18d1771a..cfe3486e5f31a 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -135,7 +135,7 @@ struct audit_nfcfgop_tab { const char *s; }; -const struct audit_nfcfgop_tab audit_nfcfgs[] = { +static const struct audit_nfcfgop_tab audit_nfcfgs[] = { { AUDIT_XT_OP_REGISTER, "register" }, { AUDIT_XT_OP_REPLACE, "replace" }, { AUDIT_XT_OP_UNREGISTER, "unregister" }, From 0090c1edebf464f34629e14ae03d764cca7e0a3b Mon Sep 17 00:00:00 2001 From: "Gustavo A. R. Silva" Date: Thu, 7 May 2020 13:50:41 -0500 Subject: [PATCH 7/8] audit: Replace zero-length array with flexible-array The current codebase makes use of the zero-length array language extension to the C90 standard, but the preferred mechanism to declare variable-length types such as these ones is a flexible array member[1][2], introduced in C99: struct foo { int stuff; struct boo array[]; }; By making use of the mechanism above, we will get a compiler warning in case the flexible array does not occur last in the structure, which will help us prevent some kind of undefined behavior bugs from being inadvertently introduced[3] to the codebase from now on. Also, notice that, dynamic memory allocations won't be affected by this change: "Flexible array members have incomplete type, and so the sizeof operator may not be applied. As a quirk of the original implementation of zero-length arrays, sizeof evaluates to zero."[1] sizeof(flexible-array-member) triggers a warning because flexible array members have incomplete type[1]. There are some instances of code in which the sizeof operator is being incorrectly/erroneously applied to zero-length arrays and the result is zero. Such instances may be hiding some bugs. So, this work (flexible-array member conversions) will also help to get completely rid of those sorts of issues. This issue was found with the help of Coccinelle. [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html [2] https://github.com/KSPP/linux/issues/21 [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour") Signed-off-by: Gustavo A. R. Silva Reviewed-by: Richard Guy Briggs Signed-off-by: Paul Moore --- include/linux/audit.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/audit.h b/include/linux/audit.h index 1a23515082113..3fcd9ee49734a 100644 --- a/include/linux/audit.h +++ b/include/linux/audit.h @@ -19,7 +19,7 @@ struct audit_sig_info { uid_t uid; pid_t pid; - char ctx[0]; + char ctx[]; }; struct audit_buffer; From 9d44a121c5a79bc8a9d67c058456bd52a83c79e7 Mon Sep 17 00:00:00 2001 From: Richard Guy Briggs Date: Wed, 20 May 2020 14:47:13 -0400 Subject: [PATCH 8/8] audit: add subj creds to NETFILTER_CFG record to Some table unregister actions seem to be initiated by the kernel to garbage collect unused tables that are not initiated by any userspace actions. It was found to be necessary to add the subject credentials to cover this case to reveal the source of these actions. A sample record: The uid, auid, tty, ses and exe fields have not been included since they are in the SYSCALL record and contain nothing useful in the non-user context. Here are two sample orphaned records: type=NETFILTER_CFG msg=audit(2020-05-20 12:14:36.505:5) : table=filter family=ipv4 entries=0 op=register pid=1 subj=kernel comm=swapper/0 type=NETFILTER_CFG msg=audit(2020-05-20 12:15:27.701:301) : table=nat family=bridge entries=0 op=unregister pid=30 subj=system_u:system_r:kernel_t:s0 comm=kworker/u4:1 Signed-off-by: Richard Guy Briggs Signed-off-by: Paul Moore --- kernel/auditsc.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/kernel/auditsc.c b/kernel/auditsc.c index cfe3486e5f31a..468a233904578 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -2557,12 +2557,18 @@ void __audit_log_nfcfg(const char *name, u8 af, unsigned int nentries, enum audit_nfcfgop op) { struct audit_buffer *ab; + char comm[sizeof(current->comm)]; ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_NETFILTER_CFG); if (!ab) return; audit_log_format(ab, "table=%s family=%u entries=%u op=%s", name, af, nentries, audit_nfcfgs[op].s); + + audit_log_format(ab, " pid=%u", task_pid_nr(current)); + audit_log_task_context(ab); /* subj= */ + audit_log_format(ab, " comm="); + audit_log_untrustedstring(ab, get_task_comm(comm, current)); audit_log_end(ab); } EXPORT_SYMBOL_GPL(__audit_log_nfcfg);