[PATCH] audit: do not panic on invalid boot parameter
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 <gedwa...@ddn.com>
---
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, ))
- 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);
pr_info("%s\n", audit_default ?
"enabled (after initialization)" : "disabled (until reboot)");
--
2.14.3
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
Re: [PATCH 1/2] audit: move processing of "audit" boot param to audit_init()
On Fri, Mar 02, 2018 at 03:33:54PM -0500, Paul Moore wrote:
> On Tue, Feb 27, 2018 at 5:52 PM, Greg Edwards <gedwa...@ddn.com> wrote:
>> So, if you want to keep the panic behavior on bad audit parameters, your
>> delayed processing should do the trick. If it instead, you are fine
>> with just pr_err and leaving audit enabled for that error case, then we
>> are almost back to my original patch, with the exceptions you previously
>> noted:
>>
>> * leave audit enabled on parsing error
>> * change panic on audit_set_enabled() failure to pr_err
>> * handle on/off as well
>
> If we get rid of the need to panic(), which I think we are all okay
> with, I think we can resolve everything with something like this, yes?
>
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 1a3e75d9a66c..d41d09e84163 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -1618,16 +1618,20 @@ 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;
> -
> - if (kstrtol(str, 0, ))
> - 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);
>
>pr_info("%s\n", audit_default ?
>"enabled (after initialization)" : "disabled (until reboot)");
>
Paul, yes this works great, and exactly what I was thinking.
Greg
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
Re: [PATCH 1/2] audit: move processing of "audit" boot param to audit_init()
On Tue, Feb 27, 2018 at 05:28:09PM -0500, Paul Moore wrote:
> On Tue, Feb 27, 2018 at 10:59 AM, Greg Edwards <gedwa...@ddn.com> wrote:
>> 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
>>> #include
>>> #include
>>> +#include
>>>
>>> #include
>>>
>>> @@ -86,6 +87,7 @@ static intaudit_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 */
>>> u32audit_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.
>
> I thought the issue was that we couldn't reliably write to the console
> in audit_enable() as it required early printks to be enabled?
You can't reliably panic from audit_enable() unless earlyprintk is
enabled, since the boot stops at the panic and the regular console isn't
initialized yet. pr_err/printk etc work fine, as those messages just
get queued up and output once the regular console is initialized (since
the boot continues on).
So, if you want to keep the panic behavior on bad audit parameters, your
delayed processing should do the trick. If it instead, you are fine
with just pr_err and leaving audit enabled for that error case, then we
are almost back to my original patch, with the exceptions you previously
noted:
* leave audit enabled on parsing error
* change panic on audit_set_enabled() failure to pr_err
* handle on/off as well
My apologies if my commit message was misleading!
Greg
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
Re: [PATCH 1/2] audit: move processing of "audit" boot param to audit_init()
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
> #include
> #include
> +#include
>
> #include
>
> @@ -86,6 +87,7 @@ static intaudit_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
> */
> u32audit_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, ))
> - 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
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
[PATCH 0/2] audit boot parameter cleanups
One of our CI tests was booting upstream kernels with the "audit=off" kernel
parameter. This was our error; it should have been "audit=0". However,
in 4.15 the verification of the boot parameter got more strict in 80ab4df62706
("audit: don't use simple_strtol() anymore"), and our errant boot parameter
value starting panic'ing the system.
The problem is this happens so early in boot, the console isn't initialized yet
and you don't see the panic message. You have no idea what the problem is
unless you add an "earlyprintk" boot option, e.g.
earlyprintk=serial,ttyS0,115200n8.
Fix this by having the boot parameter setup function just save the boot
parameter value, and process it later from a call in audit_init(). The console
is initialized by this point, and you can see any panic messages without having
to use an earlyprintk option.
Additionally, add "on" and "off" as valid audit boot parameter values.
Greg Edwards (2):
audit: move processing of "audit" boot param to audit_init()
audit: add "on"/"off" as valid boot parameter values
Documentation/admin-guide/kernel-parameters.txt | 14 +++
kernel/audit.c | 49 -
2 files changed, 39 insertions(+), 24 deletions(-)
--
2.14.3
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
[PATCH 1/2] audit: move processing of "audit" boot param to audit_init()
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 <gedwa...@ddn.com>
---
kernel/audit.c | 48 +++-
1 file changed, 31 insertions(+), 17 deletions(-)
diff --git a/kernel/audit.c b/kernel/audit.c
index 227db99b0f19..3fb11bcb4408 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -99,6 +99,9 @@ static u32audit_failure = AUDIT_FAIL_PRINTK;
/* private audit network namespace index */
static unsigned int audit_net_id;
+/* 'audit' boot parameter value */
+static char *audit_boot;
+
/**
* struct audit_net - audit private network namespace data
* @sk: communication socket
@@ -1528,11 +1531,35 @@ static struct pernet_operations audit_net_ops
__net_initdata = {
.size = sizeof(struct audit_net),
};
+/* Process kernel command-line parameter at boot time. audit=0 or audit=1. */
+static void __init audit_enable(void)
+{
+ long val;
+
+ if (!audit_boot)
+ return;
+
+ if (kstrtol(audit_boot, 0, ))
+ panic("audit: invalid 'audit' parameter value (%s)\n",
+ audit_boot);
+ audit_default = (val ? AUDIT_ON : AUDIT_OFF);
+
+ 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_info("%s\n", audit_default ?
+ "enabled (after initialization)" : "disabled (until reboot)");
+}
+
/* Initialize audit support at boot time. */
static int __init audit_init(void)
{
int i;
+ audit_enable();
+
if (audit_initialized == AUDIT_DISABLED)
return 0;
@@ -1567,26 +1594,13 @@ static int __init audit_init(void)
}
postcore_initcall(audit_init);
-/* Process kernel command-line parameter at boot time. audit=0 or audit=1. */
-static int __init audit_enable(char *str)
+/* Store kernel command-line parameter at boot time. audit=0 or audit=1. */
+static int __init audit_set(char *str)
{
- long val;
-
- if (kstrtol(str, 0, ))
- panic("audit: invalid 'audit' parameter value (%s)\n", str);
- audit_default = (val ? AUDIT_ON : AUDIT_OFF);
-
- 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_info("%s\n", audit_default ?
- "enabled (after initialization)" : "disabled (until reboot)");
-
+ audit_boot = str;
return 1;
}
-__setup("audit=", audit_enable);
+__setup("audit=", audit_set);
/* Process kernel command-line parameter at boot time.
* audit_backlog_limit= */
--
2.14.3
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
[PATCH 2/2] audit: add "on"/"off" as valid boot parameter values
Modify the "audit" boot parameter to also accept "on" or "off" as valid
parameter values. Update the documentation accordingly.
Signed-off-by: Greg Edwards <gedwa...@ddn.com>
---
Documentation/admin-guide/kernel-parameters.txt | 14 +++---
kernel/audit.c | 9 +
2 files changed, 12 insertions(+), 11 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 3fb11bcb4408..8c8304a3ea8f 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1534,15 +1534,16 @@ static struct pernet_operations audit_net_ops
__net_initdata = {
/* Process kernel command-line parameter at boot time. audit=0 or audit=1. */
static void __init audit_enable(void)
{
- long val;
-
if (!audit_boot)
return;
- if (kstrtol(audit_boot, 0, ))
+ if (!strcmp(audit_boot, "1") || !strcmp(audit_boot, "on"))
+ audit_default = AUDIT_ON;
+ else if (!strcmp(audit_boot, "0") || !strcmp(audit_boot, "off"))
+ audit_default = AUDIT_OFF;
+ else
panic("audit: invalid 'audit' parameter value (%s)\n",
audit_boot);
- audit_default = (val ? AUDIT_ON : AUDIT_OFF);
if (audit_default == AUDIT_OFF)
audit_initialized = AUDIT_DISABLED;
--
2.14.3
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
Re: [PATCH v2] audit: do not panic kernel on invalid audit parameter
On Wed, Feb 21, 2018 at 04:08:25PM -0500, Paul Moore wrote:
> On February 21, 2018 11:19:09 AM Greg Edwards <gedwa...@ddn.com> wrote:
>> If you pass in an invalid audit kernel boot parameter, e.g. 'audit=off',
>> the kernel panics very early in boot with no output on the console
>> indicating the problem.
>>
>> Instead, print the error indicating an invalid audit parameter value,
>> but leave auditing enabled.
>
> Thanks for the quick follow-up, it's actually a little *too* quick if
> I'm honest, I still haven't fully thought through all the different
> options here :)
>
> However, in the interest in capitalizing on your enthusiasm and
> willingness to help, here are some of the things I was thinking about,
> in no particular order:
>
> #1 - We might want to consider accepting both "0" and "off" as
> acceptable inputs. It should be a trivial change, and if we are going
> to default to on/enabled it seems like we should make a reasonable
> effort to do the right thing when people attempt to disable audit
> (unfortunately the kernel command line parameters seem to use both "0"
> and "off" so we can't blame people too much when they use "off").
Yes, I think this would be a good idea, and for what it's worth,
'audit=off' worked until 4.15. One of our CI tests that verifies
upstream kernels picked this up starting with 4.15.
For example, booting a 4.14.20 kernel (defconfig + kvmconfig):
[0.00] Linux version 4.14.20 (gedwards@psuche) (gcc version 7.3.1
20180130 (Red Hat 7.3.1-2) (GCC)) #1 SMP Wed Feb 21 15:14:25 M
ST 2018
[0.00] Command line: root=/dev/vda1 console=ttyS0,115200n8 audit=off
...
[0.00] Kernel command line: root=/dev/vda1 console=ttyS0,115200n8
audit=off
[0.00] audit: disabled (until reboot)
> #2 - If panic("") doesn't work, does pr_err("")? If it
> does, I would be curious to understand why.
Yes, pr_err() does work. Booting 4.16-rc2 (defconfig + kvmconfig) with
this patch:
[0.00] Linux version 4.16.0-rc2+ (gedwards@psuche) (gcc version 7.3.1
20180130 (Red Hat 7.3.1-2) (GCC)) #1 SMP Wed Feb 21 15:23:10 MST 2018
[0.00] Command line: root=/dev/vda1 console=ttyS0,115200n8 audit=off
...
[0.00] Kernel command line: root=/dev/vda1 console=ttyS0,115200n8
audit=off
[0.00] audit: invalid 'audit' parameter value (off)
[0.00] audit: enabled (after initialization)
I suspect what is happening with the current audit_enable() code is the
serial console has not been fully initialized yet by the time we call
panic(), so we never see the early printk messages queued up. I will
try and confirm.
> #3 - Related to #2 above, but there are other calls to panic() and
> pr_*() in audit_enable() that should probably be re-evaluated and
> changed. If we can't notify users/admins here, why are we trying?
I haven't looked at those other calls to panic(), but I would bet they
display the same behavior.
> #4 - Related to #2 and #3, if we can't emit messages in audit_enable()
> we need to find a way to "remember" that the user specified a bogus
> audit setting and let them know as soon as we can. One possibility
> might be to overload the audit_default variable (most places seem to
> treat it as a true/false value) with a "AUDIT_DEFAULT_INVALID" value
> (make it non-zero, say "3"?) and we could display a message in
> audit_init() or similar. Full disclosure, this *should* work ... I
> think ... but I might be missing some crucial detail.
I'm unclear why we would need this, given that #2 above does work. This
is the first time I've ever looked at the audit code, though. I was
just doing a drive-by. ;)
> I realize this is probably much more than you bargained for when you
> first submitted your patch, and if you're not interested in taking
> this any further I understand however, if you are willing to play
> a bit more I would be very grateful :)
Sure, I'm happy to look at the above.
Greg
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
[PATCH v2] audit: do not panic kernel on invalid audit parameter
If you pass in an invalid audit kernel boot parameter, e.g. 'audit=off',
the kernel panics very early in boot with no output on the console
indicating the problem.
Instead, print the error indicating an invalid audit parameter value,
but leave auditing enabled.
Fixes: 80ab4df62706 ("audit: don't use simple_strtol() anymore")
Signed-off-by: Greg Edwards <gedwa...@ddn.com>
---
Changes v1 -> v2:
- default to auditing enabled for the error case
kernel/audit.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/kernel/audit.c b/kernel/audit.c
index 227db99b0f19..9b80e9895107 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1572,8 +1572,10 @@ static int __init audit_enable(char *str)
{
long val;
- if (kstrtol(str, 0, ))
- panic("audit: invalid 'audit' parameter value (%s)\n", str);
+ if (kstrtol(str, 0, )) {
+ pr_err("invalid 'audit' parameter value (%s)\n", str);
+ val = AUDIT_ON;
+ }
audit_default = (val ? AUDIT_ON : AUDIT_OFF);
if (audit_default == AUDIT_OFF)
--
2.14.3
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
[PATCH] audit: do not panic kernel on invalid audit parameter
If you pass in an invalid audit kernel boot parameter, e.g. 'audit=off',
the kernel panics very early in boot with no output on the console
indicating the problem.
This seems overly harsh. Instead, print the error indicating an invalid
audit parameter value and leave auditing disabled.
Fixes: 80ab4df62706 ("audit: don't use simple_strtol() anymore")
Signed-off-by: Greg Edwards <gedwa...@ddn.com>
---
kernel/audit.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/kernel/audit.c b/kernel/audit.c
index 227db99b0f19..d8af7682d6a3 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1572,8 +1572,10 @@ static int __init audit_enable(char *str)
{
long val;
- if (kstrtol(str, 0, ))
- panic("audit: invalid 'audit' parameter value (%s)\n", str);
+ if (kstrtol(str, 0, )) {
+ pr_err("invalid 'audit' parameter value (%s)\n", str);
+ val = AUDIT_OFF;
+ }
audit_default = (val ? AUDIT_ON : AUDIT_OFF);
if (audit_default == AUDIT_OFF)
--
2.14.3
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
Re: [PATCH] audit: do not panic kernel on invalid audit parameter
On Tue, Feb 20, 2018 at 04:45:26PM -0500, Paul Moore wrote: > On Tue, Feb 20, 2018 at 4:33 PM, Greg Edwards <gedwa...@ddn.com> wrote: >> If you pass in an invalid audit kernel boot parameter, e.g. 'audit=off', >> the kernel panics very early in boot with no output on the console >> indicating the problem. > > I'm guessing the problem is that there was too much info dumped to the > console and the error message was lost (there is one, to say there is > "no output" isn't completely correct), is that what happened? Or was > there honestly *no* output on the console? Booting a 4.16-rc2 VM with defconfig + kvmconfig with the 'audit=off' boot parameter (my mistake), the only output you get is: . Not terribly enlightening. >> This seems overly harsh. Instead, print the error indicating an invalid >> audit parameter value and leave auditing disabled. > > There are some audit requirements which appear rather bizarre at > times, e.g. the need to panic the kernel instead of losing an audit > event. Steve is the one who follows most of these audit requirements > so I'm going to wait until he has a chance to look at this. > > There is also another issue in this patch, on error you have the audit > subsystem default to off, we may want to change this to default to on > in case of error (fail safely). Sure, that is fine. I just took a stab at what to do for the error case. I'm happy to default it to enabled, if that would be more appropriate. Greg -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit
