Address NETFILTER_CFG issues

Git context diffs were being produced with unhelpful declaration types in the place of function names to help identify the funciton in which changes were made.

Normalize x_table function declarations so that git context diff function labels work as expected.

Git context diffs were being produced with unhelpful declaration types in the place of function names to help identify the funciton in which changes were made.

Normalize ebtables function declarations so that git context diff function labels work as expected.

Signed-off-by Richard Guy Briggs <[email protected]>

(quoted diff) +6 -20

-static inline void *
-find_inlist_lock_noload(struct list_head *head, const char *name, int *error,
-			struct mutex *mutex)
+static inline void *find_inlist_lock_noload(struct list_head *head,
+					    const char *name, int *error,
+					    struct mutex *mutex)
 
-static inline struct ebt_table *
-find_table_lock(struct net *net, const char *name, int *error,
-		struct mutex *mutex)
+static inline struct ebt_table *find_table_lock(struct net *net,
 
-static inline int
-ebt_check_match(struct ebt_entry_match *m, struct xt_mtchk_param *par,
-		unsigned int *cnt)
+
+static inline int ebt_check_match(struct ebt_entry_match *m,
 
-static inline int
-ebt_check_watcher(struct ebt_entry_watcher *w, struct xt_tgchk_param *par,
-		  unsigned int *cnt)
 
-static inline int
-ebt_check_entry_size_and_hooks(const struct ebt_entry *e,
 
-static inline int
-ebt_get_udc_positions(struct ebt_entry *e, struct ebt_table_info *newinfo,
-		      unsigned int *n, struct ebt_cl_stack *udc)
 
-static inline int
-ebt_cleanup_match(struct ebt_entry_match *m, struct net *net, unsigned int *i)
 
-static inline int

Align all function declaration parameters with open parenthesis.

Git context diffs were being produced with unhelpful declaration types in the place of function names to help identify the funciton in which changes were made.

Normalize x_table function declarations so that git context diff function labels work as expected.

Signature · 2 lines

-- 
1.8.3.1

There were questions about the presence and cause of unsolicited syscall events in the logs containing NETFILTER_CFG records and sometimes unaccompanied NETFILTER_CFG records.

During testing at least the following list of events trigger NETFILTER_CFG records and the syscalls related (There may be more events that will trigger this message type.):
init_module, finit_module: modprobe
setsockopt: iptables-restore, ip6tables-restore, ebtables-restore
unshare: (h?)ostnamed
clone: libvirtd

The syscall events unsolicited by any audit rule were found to be caused by a missing !audit_dummy_context() check before creating a NETFILTER_CFG record and issuing the record immediately rather than saving the information to create the record at syscall exit.
Check !audit_dummy_context() before creating the NETFILTER_CFG record.

The vast majority of unaccompanied records are caused by the fedora default
rule: "-a never,task" and the occasional early startup one is I believe caused by the iptables filter table module hard linked into the kernel rather than a loadable module. The !audit_dummy_context() check above should avoid them.

A couple of other factors should help eliminate unaccompanied records which include commit cb74ed278f80 ("audit: always enable syscall auditing when supported and audit is enabled") which makes sure that when audit is enabled, so automatically is syscall auditing, and ghak66 which addressed initializing audit before PID 1.

Ebtables module initialization to register tables doesn't generate records because it was never hooked in to audit. Recommend adding audit hooks to log this.

Table unregistration was never logged, which is now covered.

Seemingly duplicate records are not actually exact duplicates that are caused by netfilter table initialization in different network namespaces from the same syscall. Recommend adding the network namespace ID (proc inode and dev) to the record to make this obvious (address later with ghak79 after nsid patches).

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
See: https://github.com/linux-audit/audit-kernel/issues/44

Quoted 41 lines · Richard Guy Briggs

There were questions about the presence and cause of unsolicited syscall events in the logs containing NETFILTER_CFG records and sometimes unaccompanied NETFILTER_CFG records.

During testing at least the following list of events trigger NETFILTER_CFG records and the syscalls related (There may be more events that will trigger this message type.):
init_module, finit_module: modprobe
setsockopt: iptables-restore, ip6tables-restore, ebtables-restore
unshare: (h?)ostnamed
clone: libvirtd

The syscall events unsolicited by any audit rule were found to be caused by a missing !audit_dummy_context() check before creating a NETFILTER_CFG record and issuing the record immediately rather than saving the information to create the record at syscall exit.
Check !audit_dummy_context() before creating the NETFILTER_CFG record.

The vast majority of unaccompanied records are caused by the fedora default
rule: "-a never,task" and the occasional early startup one is I believe caused by the iptables filter table module hard linked into the kernel rather than a loadable module. The !audit_dummy_context() check above should avoid them.

A couple of other factors should help eliminate unaccompanied records which include commit cb74ed278f80 ("audit: always enable syscall auditing when supported and audit is enabled") which makes sure that when audit is enabled, so automatically is syscall auditing, and ghak66 which addressed initializing audit before PID 1.

Ebtables module initialization to register tables doesn't generate records because it was never hooked in to audit. Recommend adding audit hooks to log this.

Table unregistration was never logged, which is now covered.

Seemingly duplicate records are not actually exact duplicates that are caused by netfilter table initialization in different network namespaces from the same syscall. Recommend adding the network namespace ID (proc inode and dev) to the record to make this obvious (address later with ghak79 after nsid patches).

Quoted 4 lines

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
See: https://github.com/linux-audit/audit-kernel/issues/44

What tree is this batch targeted to?

Quoted 42 lines · Pablo Neira Ayuso

Quoted 41 lines · Richard Guy Briggs

There were questions about the presence and cause of unsolicited syscall events in the logs containing NETFILTER_CFG records and sometimes unaccompanied NETFILTER_CFG records.

During testing at least the following list of events trigger NETFILTER_CFG records and the syscalls related (There may be more events that will trigger this message type.):
init_module, finit_module: modprobe
setsockopt: iptables-restore, ip6tables-restore, ebtables-restore
unshare: (h?)ostnamed
clone: libvirtd

The syscall events unsolicited by any audit rule were found to be caused by a missing !audit_dummy_context() check before creating a NETFILTER_CFG record and issuing the record immediately rather than saving the information to create the record at syscall exit.
Check !audit_dummy_context() before creating the NETFILTER_CFG record.

The vast majority of unaccompanied records are caused by the fedora default
rule: "-a never,task" and the occasional early startup one is I believe caused by the iptables filter table module hard linked into the kernel rather than a loadable module. The !audit_dummy_context() check above should avoid them.

A couple of other factors should help eliminate unaccompanied records which include commit cb74ed278f80 ("audit: always enable syscall auditing when supported and audit is enabled") which makes sure that when audit is enabled, so automatically is syscall auditing, and ghak66 which addressed initializing audit before PID 1.

Ebtables module initialization to register tables doesn't generate records because it was never hooked in to audit. Recommend adding audit hooks to log this.

Table unregistration was never logged, which is now covered.

Seemingly duplicate records are not actually exact duplicates that are caused by netfilter table initialization in different network namespaces from the same syscall. Recommend adding the network namespace ID (proc inode and dev) to the record to make this obvious (address later with ghak79 after nsid patches).

Quoted 6 lines

Quoted 4 lines

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
See: https://github.com/linux-audit/audit-kernel/issues/44

What tree is this batch targeted to?

I believe Richard was targeting this for the audit tree.

paul moore

Le 06/01/2020 à 19:54, Richard Guy Briggs a écrit :

Quoted 3 lines

Git context diffs were being produced with unhelpful declaration types in the place of function names to help identify the funciton in which changes were made.

Just for my information, how do you reproduce that? With a 'git diff'?

Quoted 3 lines

Normalize x_table function declarations so that git context diff function labels work as expected.

[snip]

Quoted 1 line

Quoted 2 lines

Signature · 1 line

1.8.3.1

git v1.8.3.1 is seven years old:
https://github.com/git/git/releases/tag/v1.8.3.1

I don't see any problems with git v2.24. Not sure that the patch brings any helpful value except complicating backports.

Nicolas

Git context diffs were being produced with unhelpful declaration types in the place of function names to help identify the funciton in which

changes were made.

Normalize x_table function declarations so that git context diff function labels work as expected.

Signed-off-by Richard Guy Briggs <[email protected]>

---
 net/netfilter/x_tables.c | 43 ++++++++++++++++++-------------------------
 1 file changed, 18 insertions(+), 25 deletions(-)

Git context diffs were being produced with unhelpful declaration types in the place of function names to help identify the funciton in which changes were made.

Normalize ebtables function declarations so that git context diff function labels work as expected.

Signed-off-by Richard Guy Briggs <[email protected]>

---
 net/bridge/netfilter/ebtables.c | 100 ++++++++++++++++++++--------------------
 1 file changed, 51 insertions(+), 49 deletions(-)

Align all function declaration parameters with open parenthesis.

Signed-off-by Richard Guy Briggs <[email protected]>

---
 net/bridge/netfilter/ebtables.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

Record the auditable parameters of any non-empty netfilter table configuration change.

See: https://github.com/linux-audit/audit-kernel/issues/25

Signed-off-by Richard Guy Briggs <[email protected]>

---
 include/linux/audit.h | 11 +++++++++++
 kernel/auditsc.c      | 16 ++++++++++++++++
 2 files changed, 27 insertions(+)

include/linux/audit.h +1 -0

index f9ceae57ca8d..96cabb095eed 100644
@@ -379,6 +379,7 @@ extern int __audit_log_bprm_fcaps(struct linux_binprm *bprm,
 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_nf_cfg(const char *name, u8 af, int nentries);
 
 static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp)
 {

include/linux/audit.h +6 -0

+}
+
 extern int audit_n_rules;
 extern int audit_signals;
 #else /* CONFIG_AUDITSYSCALL */

@@ -646,6 +653,10 @@ 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_nf_cfg(const char *name, u8 af, int nentries)
+{ }
+
 #define audit_n_rules 0
 #define audit_signals 0
 #endif /* CONFIG_AUDITSYSCALL */

kernel/auditsc.c +0 -0

index 4effe01ebbe2..4e1df4233cd3 100644

kernel/auditsc.c +6 -1

+       audit_log_format(ab, "table=%s family=%u entries=%u",
+                        name, af, nentries);
+       audit_log_end(ab);
+}
+EXPORT_SYMBOL_GPL(__audit_nf_cfg);
+
 static void audit_log_task(struct audit_buffer *ab)
 {
        kuid_t auid, uid;
--

1.8.3.1

Generate audit NETFILTER_CFG records on ebtables table registration.

Previously this was only being done for all x_tables operations and ebtables table replacement.

Call new audit_nf_cfg() to store table parameters for later use with syscall records.

Here is a sample accompanied record:
type=NETFILTER_CFG msg=audit(1494907217.558:5403): table=filter family=7 entries=0

See: https://github.com/linux-audit/audit-kernel/issues/43

Signed-off-by Richard Guy Briggs <[email protected]>

---
 net/bridge/netfilter/ebtables.c | 2 ++
 1 file changed, 2 insertions(+)

net/bridge/netfilter/ebtables.c +2 -1

index 57dc11c0f349..58126547b175 100644
@@ -1219,6 +1219,8 @@ int ebt_register_table(struct net *net, const struct ebt_table *input_table,
                *res = NULL;
        }
 
+       if (audit_enabled)
+               audit_nf_cfg(repl->name, AF_BRIDGE, repl->nentries);
        return ret;
 free_unlock:
        mutex_unlock(&ebt_mutex);
--

1.8.3.1

include/linux/audit.h extern int __audit_log_bprm_fcaps(struct linux_binprm *bprm, · line 382

Quoted 10 lines · Richard Guy Briggs

include/linux/audit.h +1 -1

index 96cabb095eed..5eab4d898c26 100644
@@ -379,7 +379,7 @@ extern int __audit_log_bprm_fcaps(struct linux_binprm *bprm,
 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_nf_cfg(const char *name, u8 af, int nentries);
+extern void __audit_nf_cfg(const char *name, u8 af, int nentries, int op);

Consider adding an enum instead of int op.

include/linux/audit.h extern int __audit_log_bprm_fcaps(struct linux_binprm *bprm, · line 384

Quoted 3 lines

include/linux/audit.h +1 -1

      if (audit_enabled)
-             audit_nf_cfg(repl->name, AF_BRIDGE, repl->nentries);
+             audit_nf_cfg(repl->name, AF_BRIDGE, repl->nentries, 1);

audit_nf_cfg(repl->name, AF_BRIDGE, repl->nentries, AUDIT_XT_OP_REPLACE);

... would be a bit more readable than '1'.

The name is just an example, you can pick something else.

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 <[email protected]>

---
 kernel/auditsc.c                | 3 ++-
 net/bridge/netfilter/ebtables.c | 3 +++
 net/netfilter/x_tables.c        | 4 +++-
 3 files changed, 8 insertions(+), 2 deletions(-)

kernel/auditsc.c +2 -1

index 999ac184246b..2644130a9e66 100644
@@ -2557,7 +2557,8 @@ void __audit_nf_cfg(const char *name, u8 af, int nentries, int op)
                return; /* audit_panic or being filtered */
        audit_log_format(ab, "table=%s family=%u entries=%u op=%s",
                         name, af, nentries,
-                        op ? "replace" : "register");
+                        op == 1 ? "replace" :
+                                  (op ? "unregister" : "register"));
        audit_log_end(ab);
 }
 EXPORT_SYMBOL_GPL(__audit_nf_cfg);

net/bridge/netfilter/ebtables.c +3 -0

index baff2f05af43..3dd4eb5b13fd 100644
@@ -1126,6 +1126,9 @@ static void __ebt_unregister_table(struct net *net, struct ebt_table *table)
        mutex_lock(&ebt_mutex);
        list_del(&table->list);
        mutex_unlock(&ebt_mutex);
+       if (audit_enabled)
+               audit_nf_cfg(table->name, AF_BRIDGE, table->private->nentries,
+                            2);
        EBT_ENTRY_ITERATE(table->private->entries, table->private->entries_size,
                          ebt_cleanup_entry, net, NULL);
        if (table->private->nentries)

net/netfilter/x_tables.c +3 -2

index 4ae4f7bf8946..e4852a0cb62f 100644
@@ -1403,7 +1403,7 @@ struct xt_table_info *xt_replace_table(struct xt_table *table,
 
        if (audit_enabled)
                audit_nf_cfg(table->name, table->af, private->number,
-                            private->number);
+                            !!private->number);
 
        return private;
 }

@@ -1466,6 +1466,8 @@ void *xt_unregister_table(struct xt_table *table)
        private = table->private;
        list_del(&table->list);
        mutex_unlock(&xt[table->af].mutex);
+       if (audit_enabled)
+               audit_nf_cfg(table->name, table->af, private->number, 2);
        kfree(table);
 
        return private;
--

1.8.3.1

Quoted 2 lines · Richard Guy Briggs

Git context diffs were being produced with unhelpful declaration types in the place of function names to help identify the funciton in which

                                                      ^^^^^^^^
                                                      function

Quoted 9 lines

changes were made.

Normalize x_table function declarations so that git context diff function labels work as expected.

Signed-off-by Richard Guy Briggs <[email protected]>

---
 net/netfilter/x_tables.c | 43 ++++++++++++++++++-------------------------
 1 file changed, 18 insertions(+), 25 deletions(-)

Considering that this patch is a style change in code outside of audit, and we want to merge this via the audit tree, I think it is best if you drop the style changes from this patchset. You can always submit them later to the netfilter developers.

paul moore

include/linux/audit.h extern int __audit_log_bprm_fcaps(struct linux_binprm *bprm, · line 382

Quoted 10 lines · Richard Guy Briggs

include/linux/audit.h +1 -1

index 96cabb095eed..5eab4d898c26 100644
@@ -379,7 +379,7 @@ extern int __audit_log_bprm_fcaps(struct linux_binprm *bprm,
 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_nf_cfg(const char *name, u8 af, int nentries);
+extern void __audit_nf_cfg(const char *name, u8 af, int nentries, int op);

Consider adding an enum instead of int op.

include/linux/audit.h extern int __audit_log_bprm_fcaps(struct linux_binprm *bprm, · line 384

Quoted 3 lines

include/linux/audit.h +1 -1

 	if (audit_enabled)
-		audit_nf_cfg(repl->name, AF_BRIDGE, repl->nentries);
+		audit_nf_cfg(repl->name, AF_BRIDGE, repl->nentries, 1);

audit_nf_cfg(repl->name, AF_BRIDGE, repl->nentries, AUDIT_XT_OP_REPLACE);

... would be a bit more readable than '1'.

The name is just an example, you can pick something else.

The default policy is NF_ACCEPT (because Rusty didn't want more email, go figure...). It occurred to me later that some table loads took a command line parameter to be able to change the default policy verdict from NF_ACCEPT to NF_DROP. In particular, filter FORWARD

hook tables. Is there a straightforward way to be able to detect this in all the audit_nf_cfg() callers to be able to log it? In particular, in:
net/bridge/netfilter/ebtables.c: ebt_register_table() net/bridge/netfilter/ebtables.c: do_replace_finish() net/bridge/netfilter/ebtables.c: __ebt_unregister_table() net/netfilter/x_tables.c: xt_replace_table() net/netfilter/x_tables.c: xt_unregister_table()

Both potential solutions are awkward, adding a parameter to pass that value in, but also trying to reach into the protocol-specific entry table to find that value. Would you have a recommendation? This assumes that reporting that default policy value is even desired or required.

Quoted 4 lines · Richard Guy Briggs

The default policy is NF_ACCEPT (because Rusty didn't want more email, go figure...). It occurred to me later that some table loads took a command line parameter to be able to change the default policy verdict from NF_ACCEPT to NF_DROP. In particular, filter FORWARD

Quoted 8 lines

hook tables. Is there a straightforward way to be able to detect this in all the audit_nf_cfg() callers to be able to log it? In particular, in:
net/bridge/netfilter/ebtables.c: ebt_register_table() net/bridge/netfilter/ebtables.c: do_replace_finish() net/bridge/netfilter/ebtables.c: __ebt_unregister_table() net/netfilter/x_tables.c: xt_replace_table() net/netfilter/x_tables.c: xt_unregister_table()

The module parameter or the policy?

The poliy can be changed via the xtables tools.
Given you can have:

*filter
:INPUT ACCEPT [0:0]
:FORWARD DROP [0:0]
:OUTPUT ACCEPT [0:0]
-A FORWARD -j ACCEPT
COMMIT

... which effectily gives a FORWARD ACCEPT policy I'm not sure logging the policy is useful.

Furthermore, ebtables has polices even for user-defined chains.

Quoted 5 lines

Both potential solutions are awkward, adding a parameter to pass that value in, but also trying to reach into the protocol-specific entry table to find that value. Would you have a recommendation? This assumes that reporting that default policy value is even desired or required.

See above, I don't think its useful. If it is needed, its probably best to define an informational struct containing the policy (accept/drop) value for the each hook points (prerouting to postrouting), fill that from the backend specific code (as thats the only place that exposes the backend specific structs ...) and then pass that to the audit logging functions.

Quoted 9 lines · Richard Guy Briggs

Generate audit NETFILTER_CFG records on ebtables table registration.

Previously this was only being done for all x_tables operations and ebtables table replacement.

Call new audit_nf_cfg() to store table parameters for later use with syscall records.

Quoted 2 lines

Here is a sample accompanied record:
type=NETFILTER_CFG msg=audit(1494907217.558:5403): table=filter family=7 entries=0

Wait a minute ... in patch 4 you have code that explicitly checks for "entries=0" and doesn't generate a record in that case; is the example a lie or is the code a lie? ;)

Quoted 14 lines

See: https://github.com/linux-audit/audit-kernel/issues/43

Signed-off-by Richard Guy Briggs <[email protected]>

---
 net/bridge/netfilter/ebtables.c | 2 ++
 1 file changed, 2 insertions(+)

net/bridge/netfilter/ebtables.c +0 -0

index 57dc11c0f349..58126547b175 100644
@@ -1219,6 +1219,8 @@ int ebt_register_table(struct net *net, const struct ebt_table *input_table,
                *res = NULL;
        }
 

Quoted 7 lines

net/bridge/netfilter/ebtables.c +2 -1

+       if (audit_enabled)
+               audit_nf_cfg(repl->name, AF_BRIDGE, repl->nentries);
        return ret;
 free_unlock:
        mutex_unlock(&ebt_mutex);
--

1.8.3.1

--
paul moore
www.paul-moore.com

Quoted 9 lines · Richard Guy Briggs

Record the auditable parameters of any non-empty netfilter table configuration change.

See: https://github.com/linux-audit/audit-kernel/issues/25

Signed-off-by Richard Guy Briggs <[email protected]>

---
 include/linux/audit.h | 11 +++++++++++
 kernel/auditsc.c      | 16 ++++++++++++++++
 2 files changed, 27 insertions(+)

I can not see a good reason why this patch is separate from patches 5 and 6, please squash them down into one patch. As it currently stands the logging function introduced here has no caller so it is pointless by itself. Strive to make an individual patch have some significance on its own whenever possible.

This will also help you write a better commit description, right now the commit description tells me nothing, but if you bring in the other patches you can talk about consolidating similar code into a common function.

Quoted 16 lines

include/linux/audit.h +1 -0

index f9ceae57ca8d..96cabb095eed 100644
@@ -379,6 +379,7 @@ extern int __audit_log_bprm_fcaps(struct linux_binprm *bprm,
 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_nf_cfg(const char *name, u8 af, int nentries);
 
 static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp)
 {

@@ -514,6 +515,12 @@ static inline void audit_ntp_log(const struct audit_ntp_data *ad)
                __audit_ntp_log(ad);
 }
 

include/linux/audit.h static inline void audit_ntp_log(const struct audit_ntp_data *ad) · line 521

Quoted 4 lines

include/linux/audit.h +4 -0

+static inline void audit_nf_cfg(const char *name, u8 af, int nentries)
+{
+       if (!audit_dummy_context())
+               __audit_nf_cfg(name, af, nentries);

See my comments below about audit_enabled.

Quoted 24 lines

include/linux/audit.h +6 -0

+}
+
 extern int audit_n_rules;
 extern int audit_signals;
 #else /* CONFIG_AUDITSYSCALL */

@@ -646,6 +653,10 @@ 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_nf_cfg(const char *name, u8 af, int nentries)
+{ }
+
 #define audit_n_rules 0
 #define audit_signals 0
 #endif /* CONFIG_AUDITSYSCALL */

kernel/auditsc.c +0 -0

index 4effe01ebbe2..4e1df4233cd3 100644
@@ -2545,6 +2545,22 @@ void __audit_ntp_log(const struct audit_ntp_data *ad)
        audit_log_ntp_val(ad, "adjust", AUDIT_NTP_ADJUST);
 }
 

kernel/auditsc.c void __audit_ntp_log(const struct audit_ntp_data *ad) · line 2548

Quoted 1 line

kernel/auditsc.c +1 -0

+void __audit_nf_cfg(const char *name, u8 af, int nentries)

Should nentries be an unsigned int?

kernel/auditsc.c void __audit_ntp_log(const struct audit_ntp_data *ad) · line 2551

Quoted 3 lines

kernel/auditsc.c +3 -0

+{
+       struct audit_buffer *ab;
+       struct audit_context *context = audit_context();

This is a good example of why the context of a caller matters; taken alone I would say that we need a check for audit_enabled here, but if we look at the latter patches we can see that the caller already has the audit_enabled check.

Considering that the caller is already doing an audit_enabled check, we might want to consider moving the audit_enabled check into audit_nf_cfg() where we do the dummy context check. It's a static inline so there shouldn't be a performance impact and it makes the caller's code cleaner.

kernel/auditsc.c void __audit_ntp_log(const struct audit_ntp_data *ad) · line 2555

Quoted 3 lines

kernel/auditsc.c +3 -0

+       if (!nentries)
+               return;
+       ab = audit_log_start(context, GFP_KERNEL, AUDIT_NETFILTER_CFG);

Why do we need the context variable, why not just call audit_context() here directly?

kernel/auditsc.c void __audit_ntp_log(const struct audit_ntp_data *ad) · line 2557

Quoted 2 lines

kernel/auditsc.c +2 -0

+       if (!ab)
+               return; /* audit_panic or being filtered */

We generally don't add comments when audit_log_start() fails elsewhere, please don't do it here.

Quoted 7 lines

kernel/auditsc.c +6 -0

+       audit_log_format(ab, "table=%s family=%u entries=%u",
+                        name, af, nentries);
+       audit_log_end(ab);
+}
+EXPORT_SYMBOL_GPL(__audit_nf_cfg);
+
 static void audit_log_task(struct audit_buffer *ab)

Quoted 4 lines

kernel/auditsc.c +0 -1

 {
        kuid_t auid, uid;
--

1.8.3.1

--
paul moore
www.paul-moore.com