Re: [PATCH] audit: set TIF_AUDIT_SYSCALL only if audit filter has been populated
On Tuesday, March 13, 2018 8:35:44 PM EDT Andy Lutomirski wrote:
On Wed, Mar 14, 2018 at 12:28 AM, Jiri Kosina
<jikos(a)kernel.org> wrote:
> On Wed, 14 Mar 2018, Andy Lutomirski wrote:
>> > Yes...I wished I was in on the beginning of this discussion. Here's
>> > the
>> > problem. We need all tasks auditable unless specifically dismissed as
>> > uninteresting. This would be a task,never rule.
>> >
>> > The way we look at it, is if it boots with audit=1, then we know
>> > auditd
>> > is expected to run at some point. So, we need all tasks to stay
>> > auditable. If they weren't and auditd enabled auditing, then we'd
need
>> > to walk the whole proctable and stab TIF_AUDIT_SYSCALL into every
>> > process in the system. It was decided that this is too ugly.
>>
>> When was that decided? That's what this patch does.
>
> I'd like to see some more justification as well.
>
> Namely, if I compare "setting TIF_AUDIT_SYSCALL for every process on a
> need-to-be-so basis" to "we always go through the slow path and
> pessimistically assume that audit is enabled and has reasonable ruleset
> loaded", I have my own (different) opinion of what is too ugly.
Me too.
That being said, on re-review of my old code, I think that
audit_dec_n_rules() may be the wrong approach. It may be better to
leave TIF_AUDIT_SYSCALL set but to make the audit code itself clear
the flag the next time through. That way we don't end up with a
partially filled in syscall audit record that never gets consumed if
we clear TIF_AUDIT_SYSCALL in the middle of a syscall.
So what happened when auditd is being restarted due to system update? Its
possible to have no auditd, no rules, and audit_enabled == 0. But its getting
ready to start a new auditd, enable audit, and load rules. In this case, we
expect tasks already dismissed by task filter to stay dismissed because the
task filter only runs at fork time.
For example, suppose httpd is not desired to be audited. The admin sets up a
task rule that sets it to never for httpd. The rule triggers at fork and
marks it inauditable. Would this patch cause httpd to suddenly become
auditable again during an auditd restart?
Rather than play run time games which is ultimately racey and prone to
missing something, what about the approach of controlling this from a boot
variable and letting user space see an error should auditing try to do its
normal thing when its not wanted? This way auditd can exit and there aren't
any races.
-Steve