Re: [PATCH] audit: do not panic on invalid boot parameter

On Mon, Mar 5, 2018 at 10:24 PM, Richard Guy Briggs <rgb(a)redhat.com&gt; wrote: > On 2018-03-05 15:05, Greg Edwards wrote: >> If you pass in an invalid audit boot parameter value, e.g. "audit=off", >> the kernel panics very early in boot before the regular console is >> initialized. Unless you have earlyprintk enabled, there is no >> indication of what the problem is on the console. >> >> Convert the panic() calls to pr_err(), and leave auditing enabled if an >> invalid parameter value was passed in. >> >> Modify the parameter to also accept "on" or "off" as valid values, and >> update the documentation accordingly. >> >> Signed-off-by: Greg Edwards <gedwards(a)ddn.com&gt; >> --- >> Changes v2 -> v3: >> - convert panic() calls to pr_err() >> - add handling of "on"/"off" as valid values >> - update documentation >> >> Changes v1 -> v2: >> - default to auditing enabled for the error case >> >> Documentation/admin-guide/kernel-parameters.txt | 14 +++++++------- >> kernel/audit.c | 21 ++++++++++++++------- >> 2 files changed, 21 insertions(+), 14 deletions(-) >> >> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt >> index 1d1d53f85ddd..0b926779315c 100644 >> --- a/Documentation/admin-guide/kernel-parameters.txt >> +++ b/Documentation/admin-guide/kernel-parameters.txt >> @@ -389,15 +389,15 @@ >> Use software keyboard repeat >> >> audit= [KNL] Enable the audit sub-system >> - Format: { "0" | "1" } (0 = disabled, 1 = enabled) >> - 0 - kernel audit is disabled and can not be enabled >> - until the next reboot >> + Format: { "0" | "1" | "off" | "on" } >> + 0 | off - kernel audit is disabled and can not be >> + enabled until the next reboot >> unset - kernel audit is initialized but disabled and >> will be fully enabled by the userspace auditd. >> - 1 - kernel audit is initialized and partially enabled, >> - storing at most audit_backlog_limit messages in >> - RAM until it is fully enabled by the userspace >> - auditd. >> + 1 | on - kernel audit is initialized and partially >> + enabled, storing at most audit_backlog_limit >> + messages in RAM until it is fully enabled by the >> + userspace auditd. >> Default: unset >> >> audit_backlog_limit= [KNL] Set the audit queue size limit. >> diff --git a/kernel/audit.c b/kernel/audit.c >> index 227db99b0f19..8fccea5ded71 100644 >> --- a/kernel/audit.c >> +++ b/kernel/audit.c >> @@ -1567,19 +1567,26 @@ static int __init audit_init(void) >> } >> postcore_initcall(audit_init); >> >> -/* Process kernel command-line parameter at boot time. audit=0 or audit=1. */ >> +/* >> + * Process kernel command-line parameter at boot time. >> + * audit={0|off} or audit={1|on}. >> + */ >> static int __init audit_enable(char *str) >> { >> - long val; >> - >> - 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 { >> + pr_err("audit: invalid 'audit' parameter value (%s)\n", str); >> + audit_default = AUDIT_ON; >> + } >> >> if (audit_default == AUDIT_OFF) >> audit_initialized = AUDIT_DISABLED; >> if (audit_set_enabled(audit_default)) >> - panic("audit: error setting audit state (%d)\n", audit_default); >> + pr_err("audit: error setting audit state (%d)\n", >> + audit_default); > > This patch looks good. On quick glance, I agree. I'll look at it a bit closer later today and likely merge it. Thanks Greg.