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

On Tue, Mar 6, 2018 at 9:38 AM, Paul Moore <paul(a)paul-moore.com&gt; wrote: > 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. It's merge now. Thanks again everyone!

paul moore