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 <[email protected]>

---
 kernel/audit.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

kernel/audit.c +4 -3

index 227db99b0f19..d8af7682d6a3 100644
@@ -1572,8 +1572,10 @@ static int __init audit_enable(char *str)
 {
        long val;
 
-       if (kstrtol(str, 0, &val))
-               panic("audit: invalid 'audit' parameter value (%s)\n", str);
+       if (kstrtol(str, 0, &val)) {
+               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

Quoted 3 lines · Greg Edwards

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?

Quoted 2 lines

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).

Quoted 9 lines · Paul Moore

Quoted 3 lines · Greg Edwards

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.

Quoted 11 lines

Quoted 2 lines

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

Quoted 3 lines · Greg Edwards

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?

Quoted 2 lines

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).

Quoted 22 lines

Fixes 80ab4df62706 ("audit: don't use simple_strtol() anymore")

Signed-off-by Greg Edwards <[email protected]>

---
 kernel/audit.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

kernel/audit.c +4 -2

index 227db99b0f19..d8af7682d6a3 100644
@@ -1572,8 +1572,10 @@ static int __init audit_enable(char *str)
 {
        long val;
 
-       if (kstrtol(str, 0, &val))
-               panic("audit: invalid 'audit' parameter value (%s)\n", str);
+       if (kstrtol(str, 0, &val)) {
+               pr_err("invalid 'audit' parameter value (%s)\n", str);
+               val = AUDIT_OFF;
+       }
        audit_default = (val ? AUDIT_ON : AUDIT_OFF);
 

Quoted 4 lines

kernel/audit.c +0 -1

        if (audit_default == AUDIT_OFF)
--

2.14.3

Signature · 7 lines

paul moore
www.paul-moore.com

--
Linux-audit mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/linux-audit

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 <[email protected]>

---
Changes v1 -> v2:
- default to auditing enabled for the error case

 kernel/audit.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

kernel/audit.c +4 -2

index 227db99b0f19..9b80e9895107 100644
@@ -1572,8 +1572,10 @@ static int __init audit_enable(char *str)
 {
 	long val;
 
-	if (kstrtol(str, 0, &val))
-		panic("audit: invalid 'audit' parameter value (%s)\n", str);
+	if (kstrtol(str, 0, &val)) {
+		pr_err("invalid 'audit' parameter value (%s)\n", str);
+		val = AUDIT_ON;
+	}
 	audit_default = (val ? AUDIT_ON : AUDIT_OFF);
 
 	if (audit_default == AUDIT_OFF)

Signature · 1 line

2.14.3

Quoted 6 lines · Greg Edwards

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").

#2 - If panic("<msg>") doesn't work, does pr_err("<msg>")? If it does, I would be curious to understand why.

#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?

#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 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 :)

Quoted 16 lines · Paul Moore

Quoted 6 lines · Greg Edwards

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:

Quoted 6 lines

#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.000000] 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.000000] Command line: root=/dev/vda1 console=ttyS0,115200n8 audit=off
...
[    0.000000] Kernel command line: root=/dev/vda1 console=ttyS0,115200n8 audit=off
[    0.000000] audit: disabled (until reboot)
                      ^^^^^^^^

Quoted 2 lines

#2 - If panic("<msg>") doesn't work, does pr_err("<msg>")? 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.000000] 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.000000] Command line: root=/dev/vda1 console=ttyS0,115200n8 audit=off
...
[    0.000000] Kernel command line: root=/dev/vda1 console=ttyS0,115200n8 audit=off
[    0.000000] audit: invalid 'audit' parameter value (off)
[    0.000000] 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.

Quoted 3 lines

#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.

Quoted 8 lines

#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. ;)

Quoted 4 lines

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

Quoted 9 lines · Greg Edwards

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 <[email protected]>

Quoted 7 lines

---

Changes v1 -> v2:
- default to auditing enabled for the error case

 kernel/audit.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

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").

#2 - If panic("<msg>") doesn't work, does pr_err("<msg>")? If it does, I would be curious to understand why.

#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?

#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 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 :)

Quoted 17 lines

kernel/audit.c +4 -2

index 227db99b0f19..9b80e9895107 100644
@@ -1572,8 +1572,10 @@ static int __init audit_enable(char *str)
 
 {
 
 	long val;
 
-	if (kstrtol(str, 0, &val))
-		panic("audit: invalid 'audit' parameter value (%s)\n", str);
+	if (kstrtol(str, 0, &val)) {
+		pr_err("invalid 'audit' parameter value (%s)\n", str);
+		val = AUDIT_ON;
+	}
 

kernel/audit.c static int __init audit_enable(char *str) · line 1581

Quoted 3 lines

kernel/audit.c +0 -0

 	audit_default = (val ? AUDIT_ON : AUDIT_OFF);
 
 	if (audit_default == AUDIT_OFF)

--
paul moore
www.paul-moore.com

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 <[email protected]>

---
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(-)

Documentation/admin-guide/kernel-parameters.txt +7 -7

index 1d1d53f85ddd..0b926779315c 100644
@@ -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.

kernel/audit.c +9 -6

index 227db99b0f19..8fccea5ded71 100644
@@ -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 {

kernel/audit.c +0 -0

 	pr_info("%s\n", audit_default ?
 		"enabled (after initialization)" : "disabled (until reboot)");

Signature · 1 line

2.14.3

Quoted 81 lines · Greg Edwards

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 <[email protected]>

---
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(-)

Documentation/admin-guide/kernel-parameters.txt +7 -7

index 1d1d53f85ddd..0b926779315c 100644
@@ -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.

kernel/audit.c +12 -6

index 227db99b0f19..8fccea5ded71 100644
@@ -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;
+     }
 

kernel/audit.c static int __init audit_init(void) · line 1589

Quoted 6 lines

kernel/audit.c +2 -1

      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.

However, I wonder if this second panic should be left alone, since it isn't related to the two audit_default options above.

audit_set_enabled() can't be sent AUDIT_LOCKED from here, there must be an error returned from looking up the security context when trying to log the config change.

There is already an audit_panic when that is
detected, but this is so early that audit_panic won't be configured to panic yet and defaults to printk. If it is also so early that no LSMs have been loaded yet then this concern is moot. There is still the question of just how useful it is to panic this early.

Quoted 5 lines

kernel/audit.c +0 -1

      pr_info("%s\n", audit_default ?
              "enabled (after initialization)" : "disabled (until reboot)");
--

2.14.3

- RGB

--
Richard Guy Briggs <[email protected]>
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

Quoted 82 lines · Richard Guy Briggs

Quoted 81 lines · Greg Edwards

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 <[email protected]>

---
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(-)

Documentation/admin-guide/kernel-parameters.txt +7 -7

index 1d1d53f85ddd..0b926779315c 100644
@@ -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.

kernel/audit.c +12 -6

index 227db99b0f19..8fccea5ded71 100644
@@ -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;
+     }
 

Quoted 8 lines

kernel/audit.c static int __init audit_init(void) · line 1589

Quoted 6 lines

kernel/audit.c +2 -1

      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.

Quoted 92 lines · Paul Moore

Quoted 90 lines · Richard Guy Briggs

Quoted 77 lines · Greg Edwards

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 <[email protected]>

---
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(-)

Documentation/admin-guide/kernel-parameters.txt +7 -7

index 1d1d53f85ddd..0b926779315c 100644
@@ -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.

kernel/audit.c +9 -6

index 227db99b0f19..8fccea5ded71 100644
@@ -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 {

kernel/audit.c static int __init audit_init(void) · line 1589

Quoted 10 lines

kernel/audit.c +5 -1

+             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.

Quoted 4 lines

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