* Re: [PATCH] audit: set TIF_AUDIT_SYSCALL only if audit filter has been populated
[not found] ` <CAHC9VhRnM0FKrHvSgf=Pm2BpQSVT2pLWWH8Z7YvG5ExBYb_Ekg@mail.gmail.com>
@ 2018-03-07 23:43 ` Paul Moore
2018-03-08 9:12 ` Richard Guy Briggs
2018-03-10 10:15 ` Steve Grubb
2018-03-08 1:06 ` Andy Lutomirski
1 sibling, 2 replies; 11+ messages in thread
From: Paul Moore @ 2018-03-07 23:43 UTC (permalink / raw)
To: Jiri Kosina, Andy Lutomirski
Cc: linux-audit, Andrew Morton, Michal Hocko, Oleg Nesterov, LKML
On Wed, Mar 7, 2018 at 6:41 PM, Paul Moore <paul@paul-moore.com> wrote:
> On Wed, Mar 7, 2018 at 11:48 AM, Jiri Kosina <jikos@kernel.org> wrote:
>> On Wed, 7 Mar 2018, Andy Lutomirski wrote:
>>> Wow, this was a long time ago.
>>
>> Oh yeah; but it now resurfaced on our side, as we are of course receiving
>> a lot of requests with respect to making syscall performance great again
>> :)
>
> Ooof. I'm not sure I can handle making more things "great again" ;)
>
>>> From memory and a bit of email diving, there are two reasons.
>>>
>>> 1. The probably was partially solved (by Oleg, IIRC) by making auditctl
>>> -a task,never cause newly spawned tasks to not suck. Yes, it's a
>>> very partial solution. After considerable nagging, I got Fedora to
>>> default to -a task,never.
>>
>> Hm, right; that's a bit inconvenient, because it takes each and every
>> vendor having to realize this option, and put it in. Making kernel do the
>> right thing automatically sounds like a better option to me.
>
> This predates audit falling into my lap, but speaking generally I
> think it would be good if the kernel did The Right Thing, so long as
> it isn't too painful.
>
>>> 2. This patch, as is, may be a bit problematic. In particular, if one
>>> task changes the audit rules while another task is in the middle of
>>> the syscall, then it's too late to audit that syscall correctly.
>>> This could be seen as a bug or it could be seen as being just fine.
>>
>> I don't think this should be a problem, given the fact that the whole
>> timing/ordering is not predictable anyway due to scheduling.
>>
>> Paul, what do you think?
>
> I'm not overly concerned about the race condition between configuring
> the audit filters and syscalls that are currently in-flight; after all
> we have that now and "fixing" it would be pretty much impractical
> (impossible maybe?). Most serious audit users configure it during
> boot and let it run, frequent runtime changes are not common as far as
> I can tell.
>
> I just looked quickly at the patch and decided it isn't something I'm
> going to be able to carefully review in the time I've got left today,
> so it's going to have to wait until tomorrow and Friday ... however,
> speaking on general principle I don't have an objection to the ideas
> put forth here.
>
> Andy, if you've got any Reviewed-by/Tested-by/NACK/etc. you want to
> add, that would be good to have.
... and I just realized that linux-audit isn't on the To/CC line,
adding them now.
Link to the patch is below.
* https://marc.info/?t=152041887600003&r=1&w=2
--
paul moore
www.paul-moore.com
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] audit: set TIF_AUDIT_SYSCALL only if audit filter has been populated
[not found] ` <CAHC9VhRnM0FKrHvSgf=Pm2BpQSVT2pLWWH8Z7YvG5ExBYb_Ekg@mail.gmail.com>
2018-03-07 23:43 ` [PATCH] audit: set TIF_AUDIT_SYSCALL only if audit filter has been populated Paul Moore
@ 2018-03-08 1:06 ` Andy Lutomirski
1 sibling, 0 replies; 11+ messages in thread
From: Andy Lutomirski @ 2018-03-08 1:06 UTC (permalink / raw)
To: Paul Moore, linux-audit
Cc: Jiri Kosina, Andy Lutomirski, Oleg Nesterov, Andrew Morton,
Michal Hocko, LKML
On Wed, Mar 7, 2018 at 11:41 PM, Paul Moore <paul@paul-moore.com> wrote:
> On Wed, Mar 7, 2018 at 11:48 AM, Jiri Kosina <jikos@kernel.org> wrote:
>> On Wed, 7 Mar 2018, Andy Lutomirski wrote:
>>> Wow, this was a long time ago.
>>
>> Oh yeah; but it now resurfaced on our side, as we are of course receiving
>> a lot of requests with respect to making syscall performance great again
>> :)
>
> Ooof. I'm not sure I can handle making more things "great again" ;)
>
>>> From memory and a bit of email diving, there are two reasons.
>>>
>>> 1. The probably was partially solved (by Oleg, IIRC) by making auditctl
>>> -a task,never cause newly spawned tasks to not suck. Yes, it's a
>>> very partial solution. After considerable nagging, I got Fedora to
>>> default to -a task,never.
>>
>> Hm, right; that's a bit inconvenient, because it takes each and every
>> vendor having to realize this option, and put it in. Making kernel do the
>> right thing automatically sounds like a better option to me.
>
> This predates audit falling into my lap, but speaking generally I
> think it would be good if the kernel did The Right Thing, so long as
> it isn't too painful.
>
>>> 2. This patch, as is, may be a bit problematic. In particular, if one
>>> task changes the audit rules while another task is in the middle of
>>> the syscall, then it's too late to audit that syscall correctly.
>>> This could be seen as a bug or it could be seen as being just fine.
>>
>> I don't think this should be a problem, given the fact that the whole
>> timing/ordering is not predictable anyway due to scheduling.
>>
>> Paul, what do you think?
>
> I'm not overly concerned about the race condition between configuring
> the audit filters and syscalls that are currently in-flight; after all
> we have that now and "fixing" it would be pretty much impractical
> (impossible maybe?). Most serious audit users configure it during
> boot and let it run, frequent runtime changes are not common as far as
> I can tell.
>
> I just looked quickly at the patch and decided it isn't something I'm
> going to be able to carefully review in the time I've got left today,
> so it's going to have to wait until tomorrow and Friday ... however,
> speaking on general principle I don't have an objection to the ideas
> put forth here.
>
> Andy, if you've got any Reviewed-by/Tested-by/NACK/etc. you want to
> add, that would be good to have.
>
It's sort of my patch, and I was reasonably happy with it, so
definitely no NACK from me. The only caveat I have is that I wrote
the original version so long ago that we need to re-audit the code.
In particular, I want to make sure that the following two cases don't
result in warnings, oopses, or other misbehavior:
1. Do a syscall with TIF_AUDIT_SYSCALL clear. Return with
TIF_AUDIT_SYSCALL set and that syscall enabled for auditing.
2.. Do a VFS syscall with TIF_AUDIT_CLEAR and have TIF_AUDIT_SYSCALL
set before we execute any VFS code. The VFS code will call into the
audit code to log the paths it touches (IIRC). Again, this shouldn't
warn or otherwise misbehave.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] audit: set TIF_AUDIT_SYSCALL only if audit filter has been populated
2018-03-07 23:43 ` [PATCH] audit: set TIF_AUDIT_SYSCALL only if audit filter has been populated Paul Moore
@ 2018-03-08 9:12 ` Richard Guy Briggs
2018-03-08 14:30 ` Andy Lutomirski
2018-03-10 10:15 ` Steve Grubb
1 sibling, 1 reply; 11+ messages in thread
From: Richard Guy Briggs @ 2018-03-08 9:12 UTC (permalink / raw)
To: Paul Moore
Cc: Jiri Kosina, Andy Lutomirski, linux-audit, Andrew Morton,
Michal Hocko, Oleg Nesterov, LKML
On 2018-03-07 18:43, Paul Moore wrote:
> On Wed, Mar 7, 2018 at 6:41 PM, Paul Moore <paul@paul-moore.com> wrote:
> > On Wed, Mar 7, 2018 at 11:48 AM, Jiri Kosina <jikos@kernel.org> wrote:
> >> On Wed, 7 Mar 2018, Andy Lutomirski wrote:
> >>> Wow, this was a long time ago.
> >>
> >> Oh yeah; but it now resurfaced on our side, as we are of course receiving
> >> a lot of requests with respect to making syscall performance great again
> >> :)
> >
> > Ooof. I'm not sure I can handle making more things "great again" ;)
> >
> >>> From memory and a bit of email diving, there are two reasons.
> >>>
> >>> 1. The probably was partially solved (by Oleg, IIRC) by making auditctl
> >>> -a task,never cause newly spawned tasks to not suck. Yes, it's a
> >>> very partial solution. After considerable nagging, I got Fedora to
> >>> default to -a task,never.
> >>
> >> Hm, right; that's a bit inconvenient, because it takes each and every
> >> vendor having to realize this option, and put it in. Making kernel do the
> >> right thing automatically sounds like a better option to me.
> >
> > This predates audit falling into my lap, but speaking generally I
> > think it would be good if the kernel did The Right Thing, so long as
> > it isn't too painful.
> >
> >>> 2. This patch, as is, may be a bit problematic. In particular, if one
> >>> task changes the audit rules while another task is in the middle of
> >>> the syscall, then it's too late to audit that syscall correctly.
> >>> This could be seen as a bug or it could be seen as being just fine.
> >>
> >> I don't think this should be a problem, given the fact that the whole
> >> timing/ordering is not predictable anyway due to scheduling.
> >>
> >> Paul, what do you think?
> >
> > I'm not overly concerned about the race condition between configuring
> > the audit filters and syscalls that are currently in-flight; after all
> > we have that now and "fixing" it would be pretty much impractical
> > (impossible maybe?). Most serious audit users configure it during
> > boot and let it run, frequent runtime changes are not common as far as
> > I can tell.
I'd agree the race condition here can't easily be fixed and isn't worth
fixing for the reasons already stated (rules don't change often and may
even be locked once in place relatively early, scheduling uncertainties).
> > I just looked quickly at the patch and decided it isn't something I'm
> > going to be able to carefully review in the time I've got left today,
> > so it's going to have to wait until tomorrow and Friday ... however,
> > speaking on general principle I don't have an objection to the ideas
> > put forth here.
The approach seems a bit draconian since it touches all tasks but only
when adding the first rule or deleting the last.
What we lose is the ability to set or clear individual task auditing and
have it stick to speed up non-audited tasks when there are rules
present, though this isn't currently used, in favour of audit_context
presence.
> > Andy, if you've got any Reviewed-by/Tested-by/NACK/etc. you want to
> > add, that would be good to have.
>
> ... and I just realized that linux-audit isn't on the To/CC line,
> adding them now.
(and Andy's non-NACK missed too...) The mailing list *is* in MAINTAINERS.
> Link to the patch is below.
>
> * https://marc.info/?t=152041887600003&r=1&w=2
>
> paul moore
- RGB
--
Richard Guy Briggs <rgb@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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] audit: set TIF_AUDIT_SYSCALL only if audit filter has been populated
2018-03-08 9:12 ` Richard Guy Briggs
@ 2018-03-08 14:30 ` Andy Lutomirski
2018-03-08 16:03 ` Richard Guy Briggs
0 siblings, 1 reply; 11+ messages in thread
From: Andy Lutomirski @ 2018-03-08 14:30 UTC (permalink / raw)
To: Richard Guy Briggs
Cc: Paul Moore, Jiri Kosina, Andy Lutomirski, linux-audit,
Andrew Morton, Michal Hocko, Oleg Nesterov, LKML
> On Mar 8, 2018, at 1:12 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
>
>> On 2018-03-07 18:43, Paul Moore wrote:
>>> On Wed, Mar 7, 2018 at 6:41 PM, Paul Moore <paul@paul-moore.com> wrote:
>>>> On Wed, Mar 7, 2018 at 11:48 AM, Jiri Kosina <jikos@kernel.org> wrote:
>>>>> On Wed, 7 Mar 2018, Andy Lutomirski wrote:
>>>>> Wow, this was a long time ago.
>>>>
>>>> Oh yeah; but it now resurfaced on our side, as we are of course receiving
>>>> a lot of requests with respect to making syscall performance great again
>>>> :)
>>>
>>> Ooof. I'm not sure I can handle making more things "great again" ;)
>>>
>>>>> From memory and a bit of email diving, there are two reasons.
>>>>>
>>>>> 1. The probably was partially solved (by Oleg, IIRC) by making auditctl
>>>>> -a task,never cause newly spawned tasks to not suck. Yes, it's a
>>>>> very partial solution. After considerable nagging, I got Fedora to
>>>>> default to -a task,never.
>>>>
>>>> Hm, right; that's a bit inconvenient, because it takes each and every
>>>> vendor having to realize this option, and put it in. Making kernel do the
>>>> right thing automatically sounds like a better option to me.
>>>
>>> This predates audit falling into my lap, but speaking generally I
>>> think it would be good if the kernel did The Right Thing, so long as
>>> it isn't too painful.
>>>
>>>>> 2. This patch, as is, may be a bit problematic. In particular, if one
>>>>> task changes the audit rules while another task is in the middle of
>>>>> the syscall, then it's too late to audit that syscall correctly.
>>>>> This could be seen as a bug or it could be seen as being just fine.
>>>>
>>>> I don't think this should be a problem, given the fact that the whole
>>>> timing/ordering is not predictable anyway due to scheduling.
>>>>
>>>> Paul, what do you think?
>>>
>>> I'm not overly concerned about the race condition between configuring
>>> the audit filters and syscalls that are currently in-flight; after all
>>> we have that now and "fixing" it would be pretty much impractical
>>> (impossible maybe?). Most serious audit users configure it during
>>> boot and let it run, frequent runtime changes are not common as far as
>>> I can tell.
>
> I'd agree the race condition here can't easily be fixed and isn't worth
> fixing for the reasons already stated (rules don't change often and may
> even be locked once in place relatively early, scheduling uncertainties).
>
>>> I just looked quickly at the patch and decided it isn't something I'm
>>> going to be able to carefully review in the time I've got left today,
>>> so it's going to have to wait until tomorrow and Friday ... however,
>>> speaking on general principle I don't have an objection to the ideas
>>> put forth here.
>
> The approach seems a bit draconian since it touches all tasks but only
> when adding the first rule or deleting the last.
>
> What we lose is the ability to set or clear individual task auditing and
> have it stick to speed up non-audited tasks when there are rules
> present, though this isn't currently used, in favour of audit_context
> presence.
>
>>> Andy, if you've got any Reviewed-by/Tested-by/NACK/etc. you want to
>>> add, that would be good to have.
>>
>> ... and I just realized that linux-audit isn't on the To/CC line,
>> adding them now.
>
> (and Andy's non-NACK missed too...) The mailing list *is* in MAINTAINERS.
>
The mailing list bounces my emails.
>> Link to the patch is below.
>>
>> * https://marc.info/?t=152041887600003&r=1&w=2
>>
>> paul moore
>
> - RGB
>
> --
> Richard Guy Briggs <rgb@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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] audit: set TIF_AUDIT_SYSCALL only if audit filter has been populated
2018-03-08 14:30 ` Andy Lutomirski
@ 2018-03-08 16:03 ` Richard Guy Briggs
0 siblings, 0 replies; 11+ messages in thread
From: Richard Guy Briggs @ 2018-03-08 16:03 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Paul Moore, Jiri Kosina, Andy Lutomirski, linux-audit,
Andrew Morton, Michal Hocko, Oleg Nesterov, LKML
On 2018-03-08 06:30, Andy Lutomirski wrote:
>
>
> > On Mar 8, 2018, at 1:12 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
> >
> >> On 2018-03-07 18:43, Paul Moore wrote:
> >>> On Wed, Mar 7, 2018 at 6:41 PM, Paul Moore <paul@paul-moore.com> wrote:
> >>>> On Wed, Mar 7, 2018 at 11:48 AM, Jiri Kosina <jikos@kernel.org> wrote:
> >>>>> On Wed, 7 Mar 2018, Andy Lutomirski wrote:
> >>>>> Wow, this was a long time ago.
> >>>>
> >>>> Oh yeah; but it now resurfaced on our side, as we are of course receiving
> >>>> a lot of requests with respect to making syscall performance great again
> >>>> :)
> >>>
> >>> Ooof. I'm not sure I can handle making more things "great again" ;)
> >>>
> >>>>> From memory and a bit of email diving, there are two reasons.
> >>>>>
> >>>>> 1. The probably was partially solved (by Oleg, IIRC) by making auditctl
> >>>>> -a task,never cause newly spawned tasks to not suck. Yes, it's a
> >>>>> very partial solution. After considerable nagging, I got Fedora to
> >>>>> default to -a task,never.
> >>>>
> >>>> Hm, right; that's a bit inconvenient, because it takes each and every
> >>>> vendor having to realize this option, and put it in. Making kernel do the
> >>>> right thing automatically sounds like a better option to me.
> >>>
> >>> This predates audit falling into my lap, but speaking generally I
> >>> think it would be good if the kernel did The Right Thing, so long as
> >>> it isn't too painful.
> >>>
> >>>>> 2. This patch, as is, may be a bit problematic. In particular, if one
> >>>>> task changes the audit rules while another task is in the middle of
> >>>>> the syscall, then it's too late to audit that syscall correctly.
> >>>>> This could be seen as a bug or it could be seen as being just fine.
> >>>>
> >>>> I don't think this should be a problem, given the fact that the whole
> >>>> timing/ordering is not predictable anyway due to scheduling.
> >>>>
> >>>> Paul, what do you think?
> >>>
> >>> I'm not overly concerned about the race condition between configuring
> >>> the audit filters and syscalls that are currently in-flight; after all
> >>> we have that now and "fixing" it would be pretty much impractical
> >>> (impossible maybe?). Most serious audit users configure it during
> >>> boot and let it run, frequent runtime changes are not common as far as
> >>> I can tell.
> >
> > I'd agree the race condition here can't easily be fixed and isn't worth
> > fixing for the reasons already stated (rules don't change often and may
> > even be locked once in place relatively early, scheduling uncertainties).
> >
> >>> I just looked quickly at the patch and decided it isn't something I'm
> >>> going to be able to carefully review in the time I've got left today,
> >>> so it's going to have to wait until tomorrow and Friday ... however,
> >>> speaking on general principle I don't have an objection to the ideas
> >>> put forth here.
> >
> > The approach seems a bit draconian since it touches all tasks but only
> > when adding the first rule or deleting the last.
> >
> > What we lose is the ability to set or clear individual task auditing and
> > have it stick to speed up non-audited tasks when there are rules
> > present, though this isn't currently used, in favour of audit_context
> > presence.
> >
> >>> Andy, if you've got any Reviewed-by/Tested-by/NACK/etc. you want to
> >>> add, that would be good to have.
> >>
> >> ... and I just realized that linux-audit isn't on the To/CC line,
> >> adding them now.
> >
> > (and Andy's non-NACK missed too...) The mailing list *is* in MAINTAINERS.
> >
>
> The mailing list bounces my emails.
They'll get approved.
> >> Link to the patch is below.
> >>
> >> * https://marc.info/?t=152041887600003&r=1&w=2
> >>
> >> paul moore
> >
> > - RGB
- RGB
--
Richard Guy Briggs <rgb@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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] audit: set TIF_AUDIT_SYSCALL only if audit filter has been populated
2018-03-07 23:43 ` [PATCH] audit: set TIF_AUDIT_SYSCALL only if audit filter has been populated Paul Moore
2018-03-08 9:12 ` Richard Guy Briggs
@ 2018-03-10 10:15 ` Steve Grubb
2018-03-14 0:22 ` Andy Lutomirski
1 sibling, 1 reply; 11+ messages in thread
From: Steve Grubb @ 2018-03-10 10:15 UTC (permalink / raw)
To: Paul Moore
Cc: Jiri Kosina, Andy Lutomirski, linux-audit, Andrew Morton,
Michal Hocko, Oleg Nesterov, LKML
On Wed, 7 Mar 2018 18:43:42 -0500
Paul Moore <paul@paul-moore.com> wrote:
> ... and I just realized that linux-audit isn't on the To/CC line,
> adding them now.
>
> Link to the patch is below.
>
> * https://marc.info/?t=152041887600003&r=1&w=2
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.
So, we need them all to be auditable if there is any intent to audit.
It doesn't matter if there are rules loaded or not. All processes have
to stay within reach.
What might be acceptable is to add one more state to audit boot variable
to indicate that auditing is never expected. We currently have:
disabled - which means we'll decide later, enabled, and immutable (no
changes allowed). Then have calls to audit_enable or loading rules
fail on that flag state so that user space can log that there is a
conflict (boot vs daemon) that has to be resolved. As long as we can
fail in a discoverable way, I think it would be OK to do something like
this. Also, I don't think we want that to be the default state at the
moment because the current default is keep all processes auditable.
-Steve
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] audit: set TIF_AUDIT_SYSCALL only if audit filter has been populated
2018-03-10 10:15 ` Steve Grubb
@ 2018-03-14 0:22 ` Andy Lutomirski
2018-03-14 0:28 ` Jiri Kosina
0 siblings, 1 reply; 11+ messages in thread
From: Andy Lutomirski @ 2018-03-14 0:22 UTC (permalink / raw)
To: Steve Grubb
Cc: Paul Moore, Jiri Kosina, Andy Lutomirski, linux-audit,
Andrew Morton, Michal Hocko, Oleg Nesterov, LKML
On Sat, Mar 10, 2018 at 10:15 AM, Steve Grubb <sgrubb@redhat.com> wrote:
> On Wed, 7 Mar 2018 18:43:42 -0500
> Paul Moore <paul@paul-moore.com> wrote:
>> ... and I just realized that linux-audit isn't on the To/CC line,
>> adding them now.
>>
>> Link to the patch is below.
>>
>> * https://marc.info/?t=152041887600003&r=1&w=2
>
> 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.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] audit: set TIF_AUDIT_SYSCALL only if audit filter has been populated
2018-03-14 0:22 ` Andy Lutomirski
@ 2018-03-14 0:28 ` Jiri Kosina
2018-03-14 0:35 ` Andy Lutomirski
2018-03-19 17:04 ` Steve Grubb
0 siblings, 2 replies; 11+ messages in thread
From: Jiri Kosina @ 2018-03-14 0:28 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Steve Grubb, Paul Moore, linux-audit, Andrew Morton, Michal Hocko,
Oleg Nesterov, LKML
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.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] audit: set TIF_AUDIT_SYSCALL only if audit filter has been populated
2018-03-14 0:28 ` Jiri Kosina
@ 2018-03-14 0:35 ` Andy Lutomirski
2018-03-19 17:15 ` Steve Grubb
2018-03-19 17:04 ` Steve Grubb
1 sibling, 1 reply; 11+ messages in thread
From: Andy Lutomirski @ 2018-03-14 0:35 UTC (permalink / raw)
To: Jiri Kosina
Cc: Andy Lutomirski, Steve Grubb, Paul Moore, linux-audit,
Andrew Morton, Michal Hocko, Oleg Nesterov, LKML
On Wed, Mar 14, 2018 at 12:28 AM, Jiri Kosina <jikos@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.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] audit: set TIF_AUDIT_SYSCALL only if audit filter has been populated
2018-03-14 0:28 ` Jiri Kosina
2018-03-14 0:35 ` Andy Lutomirski
@ 2018-03-19 17:04 ` Steve Grubb
1 sibling, 0 replies; 11+ messages in thread
From: Steve Grubb @ 2018-03-19 17:04 UTC (permalink / raw)
To: Jiri Kosina
Cc: Andy Lutomirski, Paul Moore, linux-audit, Andrew Morton,
Michal Hocko, Oleg Nesterov, LKML
On Tuesday, March 13, 2018 8:28:57 PM EDT Jiri Kosina 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.
There was some discussion about removing the flag here:
https://www.redhat.com/archives/linux-audit/2007-October/msg00053.html
-Steve
> 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.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] audit: set TIF_AUDIT_SYSCALL only if audit filter has been populated
2018-03-14 0:35 ` Andy Lutomirski
@ 2018-03-19 17:15 ` Steve Grubb
0 siblings, 0 replies; 11+ messages in thread
From: Steve Grubb @ 2018-03-19 17:15 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Michal Hocko, Jiri Kosina, Oleg Nesterov, LKML, linux-audit,
Andrew Morton
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@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
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-03-19 17:15 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <nycvar.YFH.7.76.1803071120410.15778@cbobk.fhfr.pm>
[not found] ` <CALCETrWhZ5ermtpfMt6UdLAC0M9mv7L_TsTfErGStPxznz-AWg@mail.gmail.com>
[not found] ` <nycvar.YFH.7.76.1803071745180.15778@cbobk.fhfr.pm>
[not found] ` <CAHC9VhRnM0FKrHvSgf=Pm2BpQSVT2pLWWH8Z7YvG5ExBYb_Ekg@mail.gmail.com>
2018-03-07 23:43 ` [PATCH] audit: set TIF_AUDIT_SYSCALL only if audit filter has been populated Paul Moore
2018-03-08 9:12 ` Richard Guy Briggs
2018-03-08 14:30 ` Andy Lutomirski
2018-03-08 16:03 ` Richard Guy Briggs
2018-03-10 10:15 ` Steve Grubb
2018-03-14 0:22 ` Andy Lutomirski
2018-03-14 0:28 ` Jiri Kosina
2018-03-14 0:35 ` Andy Lutomirski
2018-03-19 17:15 ` Steve Grubb
2018-03-19 17:04 ` Steve Grubb
2018-03-08 1:06 ` Andy Lutomirski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox