On Fri, Feb 23, 2018 at 11:07 AM, Richard Guy Briggs <r...@redhat.com> wrote:
> On 2018-02-22 17:22, Greg Edwards wrote:
>> 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.

Ah ha, that helps explain things - thank you!

>> 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.
>
> This part all looks good.

I had forgotten how tricky this code can be ... I believe there are a
few issues with patch 1/2 and how it initializes audit (it breaks
auditing for PID 1), but I need to double check a few things first.

>> Additionally, add "on" and "off" as valid audit boot parameter values.
>
> This part is a step in the right direction, but I've got minor concerns
> about variations on "0" and "1" that will no longer work, since any
> non-zero integer worked previously and will no longer do so.
>
> I would have still used the integer conversion but checked explicitly
> for "on" and "off" prior to testing for an integer.

I agree with Richard that testing for "on"/"off" independently, and
first, would be a good idea.

I also just realized that we probably can't default to enabling audit,
at least not how I was thinking anyway.

Anyway, a bit of an apology, I had hoped to review this before I ended
my day today, but I didn't leave myself enough time ... I'll try to
provide proper feedback this weekend, if that doesn't happen you
should see something early next week.

Thanks again for your help thus far, additional hands are always welcome!

>> 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(-)
>
> - RGB
>
> --
> Richard Guy Briggs <r...@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
>
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit

-- 
paul moore
www.paul-moore.com

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit

Reply via email to