[PATCH 1/2] audit: move processing of "audit" boot param to audit_init()
Greg Edwards
gedwards at ddn.com
Tue Feb 27 15:59:21 UTC 2018
On Mon, Feb 26, 2018 at 07:00:51PM -0500, Paul Moore wrote:
>
> 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;
> + }
> +
If you are just going to pr_err() on invalid audit parameter instead of
panic, you don't need AUDIT_ARGERR at all or the delayed error reporting
of it here. You can just use pr_err() in audit_enable() directly.
> 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;
Just pr_err() here and set audit_default = AUDIT_ON for the error case.
>
> - 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);
You could leave this here if you did error
reporting/audit_default=AUDIT_ON in audit_enable() directly.
> -
> - pr_info("%s\n", audit_default ?
> - "enabled (after initialization)" : "disabled (until reboot)");
> + }
>
> return 1;
> }
Another idea I had was switching those original panic() calls to
audit_panic(), and then making audit_failure another __setup option,
i.e. audit_failure={silent,printk,panic} corresponding to
AUDIT_FAIL_{SILENT,PRINTK,PANIC}. You could default it to
AUDIT_FAIL_PRINTK as it is today. Users that really cared could boot
with audit_failure=panic. I don't know if that would be overloading
audit_panic() outside of its intended purpose, though.
Greg
More information about the Linux-audit
mailing list