[PATCH] audit: do not panic kernel on invalid audit parameter

[PATCH] audit: do not panic kernel on invalid audit parameter

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

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 <gedwards@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, &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

Re: [PATCH] audit: do not panic kernel on invalid audit parameter

[Paul Moore]

On Tue, Feb 20, 2018 at 4:33 PM, Greg Edwards <gedwards@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?

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

> Fixes: 80ab4df62706 ("audit: don't use simple_strtol() anymore")
> Signed-off-by: Greg Edwards <gedwards@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, &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
>



-- 
paul moore
www.paul-moore.com

Re: [PATCH] audit: do not panic kernel on invalid audit parameter

[Greg Edwards]

On Tue, Feb 20, 2018 at 04:45:26PM -0500, Paul Moore wrote:
> On Tue, Feb 20, 2018 at 4:33 PM, Greg Edwards <gedwards@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

Re: [PATCH] audit: do not panic kernel on invalid audit parameter

[Paul Moore]

On Tue, Feb 20, 2018 at 5:00 PM, Greg Edwards <gedwards@ddn.com> wrote:
> On Tue, Feb 20, 2018 at 04:45:26PM -0500, Paul Moore wrote:
>> On Tue, Feb 20, 2018 at 4:33 PM, Greg Edwards <gedwards@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.

Oooo, fun.

I wonder if the call to panic() is happening before the kernel
initializes the console.  Ungh.

I'll have to play with this some, but if that is the case we may not
be able to display anything meaningful, and we may just have to
default to "on".

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

-- 
paul moore
www.paul-moore.com

Re: [PATCH] audit: do not panic kernel on invalid audit parameter

[Richard Guy Briggs]

On 2018-02-20 16:45, Paul Moore wrote:
> On Tue, Feb 20, 2018 at 4:33 PM, Greg Edwards <gedwards@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?
> 
> > 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).

Like Paul, I would have to support the default to on in case of error.

> > Fixes: 80ab4df62706 ("audit: don't use simple_strtol() anymore")
> > Signed-off-by: Greg Edwards <gedwards@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, &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
> >
> 
> 
> 
> -- 
> paul moore
> www.paul-moore.com
> 
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit

- RGB

--
Richard Guy Briggs <rgb@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

[PATCH v2] audit: do not panic kernel on invalid audit parameter

[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 <gedwards@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, &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)
-- 
2.14.3

Re: [PATCH v2] audit: do not panic kernel on invalid audit parameter

[Paul Moore]

On February 21, 2018 11:19:09 AM Greg Edwards <gedwards@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.
>
> Fixes: 80ab4df62706 ("audit: don't use simple_strtol() anymore")
> Signed-off-by: Greg Edwards <gedwards@ddn.com>
> ---
> 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 :)

> 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, &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)
> -- 
> 2.14.3

--
paul moore
www.paul-moore.com

Re: [PATCH v2] audit: do not panic kernel on invalid audit parameter

[Greg Edwards]

On Wed, Feb 21, 2018 at 04:08:25PM -0500, Paul Moore wrote:
> On February 21, 2018 11:19:09 AM Greg Edwards <gedwards@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.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)
                      ^^^^^^^^

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

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

Re: [PATCH v2] audit: do not panic kernel on invalid audit parameter

[Steve Grubb]

On Wednesday, February 21, 2018 4:08:25 PM EST Paul Moore wrote:
> On February 21, 2018 11:19:09 AM Greg Edwards <gedwards@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.
> > 
> > Fixes: 80ab4df62706 ("audit: don't use simple_strtol() anymore")
> > Signed-off-by: Greg Edwards <gedwards@ddn.com>
> > ---
> > 
> > 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.

Well, auditd will probably have a big problem starting up and that should be 
a big clue. Also, this could be remembered in a way that a fault indication 
is returned by auditctl -s? Loading audit rules leads to checking audit 
status which journald keeps around.

-Steve

> 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 :)
> > 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, &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)
> 
> --
> paul moore
> www.paul-moore.com

Re: [PATCH v2] audit: do not panic kernel on invalid audit parameter

[Richard Guy Briggs]

On 2018-02-21 15:51, Greg Edwards wrote:
> On Wed, Feb 21, 2018 at 04:08:25PM -0500, Paul Moore wrote:
> > On February 21, 2018 11:19:09 AM Greg Edwards <gedwards@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.

Huh, at first I wondered if the earlier audit init was at play here, but
now I'm suspecting
	80ab4df62706b882922c3bb0b05ce2c8ab10828a
	("audit: don't use simple_strtol() anymore")
is the primary culprit, exacerbated by earlier init from the same
patchset.

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

- RGB

--
Richard Guy Briggs <rgb@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

[PATCH] audit: do not panic on invalid boot parameter

[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 <gedwards@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, &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;
+	}
 
 	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

Re: [PATCH] audit: do not panic on invalid boot parameter

[Richard Guy Briggs]

On 2018-03-05 15:05, Greg Edwards wrote:
> 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 <gedwards@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, &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;
> +	}
>  
>  	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.

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

- RGB

--
Richard Guy Briggs <rgb@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

Re: [PATCH] audit: do not panic on invalid boot parameter

[Paul Moore]

On Mon, Mar 5, 2018 at 10:24 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2018-03-05 15:05, Greg Edwards wrote:
>> 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 <gedwards@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, &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;
>> +     }
>>
>>       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.

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

There is still the silent-panic problem that started this entire
discussion/patch.  If we really need to panic() here (and I currently
don't think we need to panic), then we need to devise another solution
(see the previous patches that we discussed).

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

Keep in mind that we aren't actually logging anything here as
audit_initialized isn't set to AUDIT_INITIALIZED yet.  We're using
audit_set_enabled() simply so we consolidate all of the enable/disable
code in one place (see the original commit where I switched it over to
use audit_set_enabled()).

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

Not an issue, we are never going to get past audit_log_start() as we
haven't initialized the audit subsystem yet.

>>       pr_info("%s\n", audit_default ?
>>               "enabled (after initialization)" : "disabled (until reboot)");
>> --
>> 2.14.3
>>
>
> - RGB
>
> --
> Richard Guy Briggs <rgb@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

-- 
paul moore
www.paul-moore.com

Re: [PATCH] audit: do not panic on invalid boot parameter

[Paul Moore]

On Tue, Mar 6, 2018 at 9:38 AM, Paul Moore <paul@paul-moore.com> wrote:
> On Mon, Mar 5, 2018 at 10:24 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
>> On 2018-03-05 15:05, Greg Edwards wrote:
>>> 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 <gedwards@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, &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;
>>> +     }
>>>
>>>       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.

It's merge now.  Thanks again everyone!

-- 
paul moore
www.paul-moore.com

Re: [PATCH] audit: do not panic on invalid boot parameter

[Richard Guy Briggs]

On 2018-03-06 13:53, Paul Moore wrote:
> On Tue, Mar 6, 2018 at 9:38 AM, Paul Moore <paul@paul-moore.com> wrote:
> > On Mon, Mar 5, 2018 at 10:24 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> >> On 2018-03-05 15:05, Greg Edwards wrote:
> >>> 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 <gedwards@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, &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;
> >>> +     }
> >>>
> >>>       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.
> 
> It's merge now.  Thanks again everyone!

If you haven't already, please add my Reviewed-by:

> paul moore

- RGB

--
Richard Guy Briggs <rgb@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