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

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.