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