On 2018-02-23 18:58, Paul Moore wrote:
> 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.

I checked (though didn't test) that and I had believed it was fine since
postcore_initcall still determines when audit_init().  However, this 
does move audit_enable initialization back to audit_init() in the
initcalls list from its place in __setup() which effectively reverts the
change made in 173743dd99a49c956b124a74c8aacb0384739a4c ("audit: ensure
that 'audit=1' actually enables audit for PID 1") that corrected the
early init messages lost issue.
(See: https://github.com/linux-audit/audit-kernel/issues/66)

Moving from __initcall() to postcore_initcall() still happens after PID
1 init, so this is irrelevant as I should have remembered from last
Hallowe'en's discussion.

So, I agree with Paul that the initialization of audit_enable must be
kept in the call from __setup() so that by the time PID 1 is launched
before any of the initcalls, its value can allow TIF_SYSCALL_AUDIT to be
set in task initialization and permit PID 1 to be audited.  Greg, I
originally pictured you leaving audit_enable set in that __setup() call
and store the kernel init audit= command line parameter and a flag (I
suppose the non-NULL pointer to the audit= parameter would do that) and
a specific message to report later if there was an error.

So, with a nod to Paul, I'll retract my "looks good".

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

- 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

Reply via email to