Re: [PATCH 1/2] audit: move processing of "audit" boot param to audit_init()

On Thu, Feb 22, 2018 at 7:22 PM, Greg Edwards <gedwards(a)ddn.com&gt; wrote: > The processing of the "audit" boot parameter is handled before the > console has been initialized. We therefore miss any panic messages if > we fail to verify the boot parameter or set the audit state, unless we > also enable earlyprintk. > > Instead, have the boot parameter function just save the parameter value > and process it later from audit_init(), which is a postcore_initcall() > function. > > Signed-off-by: Greg Edwards <gedwards(a)ddn.com&gt; > --- > kernel/audit.c | 48 +++++++++++++++++++++++++++++++----------------- > 1 file changed, 31 insertions(+), 17 deletions(-) In the process of trying to explain things a bit further (see the discussion thread in 0/2), I realized that some example code might speak better than I could. Below is what I was thinking for a fix; I haven't tested it, so it may blow up badly, but hopefully it makes things a bit more clear. One thing of note, I did away with the kstrtol() altogether, when we are only looking for zero and one it seems easier to just compare the strings.

diff --git a/kernel/audit.c b/kernel/audit.c index 1a3e75d9a66c..5dd63f60ef90 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -61,6 +61,7 @@ #include <linux/gfp.h> #include <linux/pid.h> #include <linux/slab.h> +#include <linux/string.h> #include <linux/audit.h> @@ -86,6 +87,7 @@ static int audit_initialized; #define AUDIT_OFF 0 #define AUDIT_ON 1 #define AUDIT_LOCKED 2 +#define AUDIT_ARGERR 3 /* indicate a "audit=X" syntax error at boot */ u32 audit_enabled = AUDIT_OFF; bool audit_ever_enabled = !!AUDIT_OFF; @@ -1581,6 +1583,12 @@ static int __init audit_init(void) if (audit_initialized == AUDIT_DISABLED) return 0; + /* handle any delayed error reporting from audit_enable() */ + if (audit_default == AUDIT_ARGERR) { + pr_err("invalid 'audit' parameter value, use 0 or 1\n"); + audit_default = AUDIT_ON; + } + audit_buffer_cache = kmem_cache_create("audit_buffer", sizeof(struct audit_buffer), 0, SLAB_PANIC, NULL); @@ -1618,19 +1626,23 @@ postcore_initcall(audit_init); /* Process kernel command-line parameter at boot time. audit=0 or audit=1. */ static int __init audit_enable(char *str) { - long val; + /* NOTE: we can't reliably send any messages to the console here */ - if (kstrtol(str, 0, &val)) - panic("audit: invalid 'audit' parameter value (%s)\n", str); - audit_default = (val ? AUDIT_ON : AUDIT_OFF); + if (!strcasecmp(str, "off") || !strcmp(str, "0")) + audit_default = AUDIT_OFF; + else if (!strcasecmp(str, "on") || !strcmp(str, "1")) + audit_default = AUDIT_ON; + else + audit_default = AUDIT_ARGERR; - if (audit_default == AUDIT_OFF) + if (audit_default) { + audit_enabled = AUDIT_ON; + audit_ever_enabled = AUDIT_ON; + } else { + audit_enabled = AUDIT_OFF; + audit_ever_enabled = AUDIT_OFF; audit_initialized = AUDIT_DISABLED; - if (audit_set_enabled(audit_default)) - panic("audit: error setting audit state (%d)\n", audit_default); - - pr_info("%s\n", audit_default ? - "enabled (after initialization)" : "disabled (until reboot)"); + } return 1; } -- paul moore www.paul-moore.com -- Linux-audit mailing list Linux-audit(a)redhat.com https://www.redhat.com/mailman/listinfo/linux-audit