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

[Richard Guy Briggs]

On 2016-12-07 10:05, Paul Moore wrote:
> On Tue, Dec 6, 2016 at 10:32 PM, Richard Guy Briggs <rgb at redhat.com> wrote:
> > On 2016-12-06 19:17, Paul Moore wrote:
> >> On Tue, Dec 6, 2016 at 12:13 AM, Richard Guy Briggs <rgb at redhat.com> wrote:
> >> > On 2016-12-05 12:48, Paul Moore wrote:
> >> >> On Mon, Dec 5, 2016 at 11:52 AM, Richard Guy Briggs <rgb at redhat.com> wrote:
> >> >> > On 2016-12-05 11:02, Paul Moore wrote:
> >> >> >> On Mon, Dec 5, 2016 at 3:02 AM, Richard Guy Briggs <rgb at redhat.com> wrote:
> >> >> >> > Add a method to reset the audit_lost value.
> >> >> >> >
> >> >> >> > An AUDIT_GET message will get the current audit_lost value and reset the
> >> >> >> > counter to zero iff (if and only if) the AUDIT_FEATURE_LOST_RESET
> >> >> >> > feature is set.
> >> >> >> >
> >> >> >> > If the flag AUDIT_FEATURE_BITMAP_LOST_RESET is present in the audit
> >> >> >> > feature bitmap, the feature is settable by setting the
> >> >> >> > AUDIT_FEATURE_LOST_RESET flag in the audit feature list with an
> >> >> >> > AUDIT_SET_FEATURE call.  This setting is lockable.
> >> >> >> >
> >> >> >> > See: https://github.com/linux-audit/audit-kernel/issues/3
> >> >> >> >
> >> >> >> > Signed-off-by: Richard Guy Briggs <rgb at redhat.com>
> >> >> >> > ---
> >> >> >> > Note: The AUDIT_FEATURE_BITMAP_LOST_RESET check may not be necessary if
> >> >> >> > it is possible to read all the entries from audit_feature_names from
> >> >> >> > userspace.
> >> >> >> > ---
> >> >> >> >  include/uapi/linux/audit.h |    7 +++++--
> >> >> >> >  kernel/audit.c             |    9 ++++++---
> >> >> >> >  2 files changed, 11 insertions(+), 5 deletions(-)
> >> >> >>
> >> >> >> Instead of resetting the lost counter on an AUDIT_GET if the reset
> >> >> >> feature is set, how about preserving the AUDIT_GET behavior, skipping
> >> >> >> the AUDIT_FEATURE_* addition, and simply reset the lost value by
> >> >> >> sending a AUDIT_SET message with AUDIT_STATUS_LOST (you obviously have
> >> >> >> to add this to the uapi header).
> >> >> >
> >> >> > I realized as I was coding it up that we would potentially lose an
> >> >> > accurate count if the read and reset were not atomic.  This was the
> >> >> > reason for using atomic_xchg().
> >> >>
> >> >> Well, okay, but that isn't what I was talking about ... ?  The
> >> >> audit_cmd_mutex is held for both AUDIT_GET and AUDIT_SET so we should
> >> >> never process these requests at the same time.
> >> >
> >> > I guess I still don't follow what you are talking about...  I hadn't
> >> > thought of including both an AUDIT_GET and an AUDIT_SET in the same skb,
> >> > but that would ensure that no other command reads or resets the lost
> >> > value.  However, the lost value could still be incrementing on another
> >> > CPU between these two commands, losing an accurate lost count.
> >>
> >> 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 agree the AUDIT_SET approach is much more elegant, which is the
original way I had envisioned doing it, but it is lossy, and I'd lean
towards accuracy and reliability than uncertainty and doubt.

> 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.
> 
> -- 
> paul moore
> www.paul-moore.com

- RGB

--
Richard Guy Briggs <rgb at redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635