On Fri, Mar 2, 2018 at 4:46 PM, Greg Edwards <gedwa...@ddn.com> wrote:
> On Fri, Mar 02, 2018 at 03:33:54PM -0500, Paul Moore wrote:
>> On Tue, Feb 27, 2018 at 5:52 PM, Greg Edwards <gedwa...@ddn.com> wrote:
>>> So, if you want to keep the panic behavior on bad audit parameters, your
>>> delayed processing should do the trick.  If it instead, you are fine
>>> with just pr_err and leaving audit enabled for that error case, then we
>>> are almost back to my original patch, with the exceptions you previously
>>> noted:
>>>
>>>   * leave audit enabled on parsing error
>>>   * change panic on audit_set_enabled() failure to pr_err
>>>   * handle on/off as well
>>
>> If we get rid of the need to panic(), which I think we are all okay
>> with, I think we can resolve everything with something like this, yes?
>>
>> diff --git a/kernel/audit.c b/kernel/audit.c
>> index 1a3e75d9a66c..d41d09e84163 100644
>> --- a/kernel/audit.c
>> +++ b/kernel/audit.c
>> @@ -1618,16 +1618,20 @@ postcore_initcall(audit_init);
>> /* Process kernel command-line parameter at boot time.  audit=0 or audit=1. 
>> */
>> 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)");
>>
>
> Paul, yes this works great, and exactly what I was thinking.

Great, sorry it took so long to get to this point, but I'm glad we've
finally synced up.

Would you the honor, and the glory (oh the glory!) of submitting a
formal patch? ;)

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