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

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.

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

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.