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

[Richard Guy Briggs]

On 2018-02-26 19:00, Paul Moore wrote:
> On Thu, Feb 22, 2018 at 7:22 PM, Greg Edwards <gedwards at ddn.com> 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 at ddn.com>
> > ---
> >  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.

It is easier, but might break stuff that previously worked, such as any
non-zero integer to turn the feature on.  This isn't documented to work
but then neither was "off" and "on" which are now being accomodated.

Also, keeping a pointer to the offending string would be helpful in the
error reporting text to show exactly what the parser thinks it sees.

> 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 at redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit

- RGB

--
Richard Guy Briggs <rgb at redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635