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

[Paul Moore]

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.

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