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

On Tue, Dec 6, 2016 at 12:13 AM, Richard Guy Briggs <rgb(a)redhat.com&gt; wrote: > On 2016-12-05 12:48, Paul Moore wrote: >> On Mon, Dec 5, 2016 at 11:52 AM, Richard Guy Briggs <rgb(a)redhat.com&gt; wrote: >> > On 2016-12-05 11:02, Paul Moore wrote: >> >> On Mon, Dec 5, 2016 at 3:02 AM, Richard Guy Briggs <rgb(a)redhat.com&gt; 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(a)redhat.com&gt; >> >> > --- >> >> > 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?

paul moore