Re: [RFC][PATCH] audit: add feature audit_lost reset

On Wed, Dec 7, 2016 at 6:30 PM, Steve Grubb <sgrubb(a)redhat.com&gt; wrote: > On Wednesday, December 7, 2016 6:10:49 PM EST Paul Moore wrote: >> On Wed, Dec 7, 2016 at 10:58 AM, Richard Guy Briggs <rgb(a)redhat.com&gt; wrote: >> > On 2016-12-07 10:53, Steve Grubb wrote: >> >> On Wednesday, December 7, 2016 10:05:30 AM EST Paul Moore wrote: >> >> > On Tue, Dec 6, 2016 at 10:32 PM, Richard Guy Briggs <rgb(a)redhat.com&gt; > wrote: >> >> > > On 2016-12-06 19:17, Paul Moore wrote: >> >> > >> On Tue, Dec 6, 2016 at 12:13 AM, Richard Guy Briggs <rgb(a)redhat.com&gt; >> >> > >> Okay, back up ... this whole mess about atomic_xchg() was always >> >> > >> unrelated to my original suggestion, let's focus on my original >> >> > >> comment ... don't reset the counter on a AUDIT_GET, reset it on a >> >> > >> AUDIT_SET with an AUDIT_STATUS_LOST, does that make sense? >> >> > > >> >> > > I understood that. It sounds like a nice simple and straightforward >> >> > > method to do it but for the question of accuracy. Please rewind to >> >> > > my >> >> > > fundamental point: How do we get an accurate reading of the last >> >> > > value >> >> > > of audit_lost before resetting it? >> >> > >> >> > Okay, I thought you were worried about a different race, which is why >> >> > this discussion wasn't making much sense to me. I understand your >> >> > point, but I really dislike the API; although that's not your fault, >> >> > it's really the only way to do it via AUDIT_GET. >> >> > >> >> > I'd much prefer we go with the cleaner AUDIT_SET approach and just not >> >> > worry about the small race window. It would only be an issue if you >> >> > reset the count under heavy audit load, and why would you reset the >> >> > lost value if you were under a heavy audit load? That just doesn't >> >> > make sense. >> >> > >> >> > I suppose we should hear from Steve on this since he was the one who >> >> > has been asking for this feature, although I'm pretty sure I know what >> >> > he is going to say. >> >> >> >> To start with, this request comes from users of the audit system. I just >> >> passed along the request. The issue is that when you do auditctl -s, you >> >> get the number of records lost. If you do it the next day, you have to >> >> do math to see what the one day delta is. So, to make reporting easy, >> >> they want it to be reset whenever they do audictl -s. >> >> >> >> You could also make a AUDIT_GET_RESET that gets the status and resets the >> >> number atomically. Then I can add another commandline option to auditctl >> >> that allows an admin to say also reset the counters. If that command >> >> line option is passed, I call AUDIT_GET_RESET otherwise I call >> >> AUDIT_GET. Thought?> >> > This would be slightly simpler in kernel implementation than the method >> > I proposed and would work fine, off the top of my head. >> >> I'd prefer not to introduce another command message type for something >> small like this. >> >> Steve, do you have any objection to the AUDIT_SET based approach? > > Either way, we'd need a feature flag so that I can tell if the kernel supports > this or not. I think we are okay without a specific feature flag as sending a AUDIT_SET/reset on an old kernel will be harmless; it won't do anything, but it shouldn't return an error either.

> Also, it should accept "0" as the only valid value. Of course.

> We can do this and I can make auditctl do the two back to back before displaying the > results to minimize the window of risk.

>> Based on what you've said above, it would seem like the potential race >> condition with AUDIT_SET wouldn't be a significant issue. > > All a matter of perspective. What I think is a reasonable risk someone else > may disagree. Does anyone else on the list object? If not I'd say go with it. Judgement calls are always a matter of perspective, since you are the only one who has asked for this (even if it is by proxy) I was asking for your perspective. Anyway, it looks like we're probably okay here; if we don't hear anything in the next day or two lets go ahead with the AUDIT_SET approach. If it proves to be a problem we can always introduce one of the other approaches later.

paul moore