On Wednesday, February 21, 2018 4:08:25 PM EST Paul Moore wrote: > On February 21, 2018 11:19:09 AM Greg Edwards <gedwa...@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 <gedwa...@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 -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit