12:56 p.m.
On Friday, December 16, 2016 5:58:39 PM EST Paul Moore wrote:
On Fri, Dec 16, 2016 at 1:59 AM, Richard Guy Briggs
<rgb(a)redhat.com> wrote:
> Add a method to reset the audit_lost value.
>
> An AUDIT_SET message with the AUDIT_STATUS_LOST flag set by itself
> will return a positive value repesenting the current audit_lost value
> and reset the counter to zero. If AUDIT_STATUS_LOST is not the
> only flag set, the reset command will be ignored. The value sent with
> the command is ignored.
>
> An AUDIT_LOST_RESET message will be queued to the listening audit
> daemon. The message will be similar to a CONFIG_CHANGE message with the
> fields "lost=0" and "old=" containing the value of audit_lost at
reset
> ending with a successful result code.
>
> See: https://github.com/linux-audit/audit-kernel/issues/3
>
> Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com>
> ---
> v3: Switch, from returing a message to the initiating process, to
> queueing the message for the audit log.
>
> v2: Switch from AUDIT_GET to AUDIT_SET, adding a +ve return code and
> sending a dedicated AUDIT_LOST_RESET message.
> ---
>
> include/uapi/linux/audit.h | 2 ++
> kernel/audit.c | 16 +++++++++++++++-
> 2 files changed, 17 insertions(+), 1 deletions(-)
>
> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> index 208df7b..6d38bff 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -70,6 +70,7 @@
>
> #define AUDIT_TTY_SET 1017 /* Set TTY auditing status */
> #define AUDIT_SET_FEATURE 1018 /* Turn an audit feature on or off
> */ #define AUDIT_GET_FEATURE 1019 /* Get which features are
> enabled */>
> +#define AUDIT_LOST_RESET 1020 /* Reset the audit_lost value */
The 1000 block is for command and control, not logging. If this is used in
logging, it should be in the 1300 block. But see below, this probably is not
needed.
> #define AUDIT_FIRST_USER_MSG 1100 /* Userspace messages
mostly
> uninteresting to kernel */ #define AUDIT_USER_AVC 1107 /* We
> filter this differently */>
> @@ -325,6 +326,7 @@ enum {
>
> #define AUDIT_STATUS_RATE_LIMIT 0x0008
> #define AUDIT_STATUS_BACKLOG_LIMIT 0x0010
> #define AUDIT_STATUS_BACKLOG_WAIT_TIME 0x0020
>
> +#define AUDIT_STATUS_LOST 0x0040
>
> #define AUDIT_FEATURE_BITMAP_BACKLOG_LIMIT 0x00000001
> #define AUDIT_FEATURE_BITMAP_BACKLOG_WAIT_TIME 0x00000002
>
> diff --git a/kernel/audit.c b/kernel/audit.c
> index f1ca116..441e8c0 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -122,7 +122,7 @@
>
> 3) suppressed due to audit_rate_limit
> 4) suppressed due to audit_backlog_limit
>
> */
>
> -static atomic_t audit_lost = ATOMIC_INIT(0);
> +static atomic_t audit_lost = ATOMIC_INIT(0);
>
> /* The netlink socket. */
> static struct sock *audit_sock;
>
> @@ -920,6 +920,20 @@ static int audit_receive_msg(struct sk_buff *skb,
> struct nlmsghdr *nlh)>
> if (err < 0)
>
> return err;
>
> }
>
> + if (s.mask == AUDIT_STATUS_LOST) {
> + struct audit_buffer *ab;
> + u32 lost = atomic_xchg(&audit_lost, 0);
> +
> + ab = audit_log_start(NULL, GFP_KERNEL,
> AUDIT_LOST_RESET); + if (unlikely(!ab))
> + return lost;
I'm generally not a fan of adding the likely/unlikely optimizations in
non-critial/control path code like this one, but don't respin the
patch just for this. However, if you do have to respin the patch
please fix this.
> + audit_log_format(ab, "lost=0 old=%u", lost);
> + audit_log_session_info(ab);
> + audit_log_task_context(ab);
> + audit_log_format(ab, " res=1");
We're still need to userspace code, so no rush on this, but we should
get Steve's opinion on the format; he'll surely have something to say.
So, I'm looking at this and wondering a few things. The config option right
above this is audit_set_backlog_wait_time. Wouldn't you want to pattern this
after it? IOW, make an audit_reset_lost function which calls
audit_do_config_change() which calls audit_log_config_change()? This would make
the event type CONFIG_CHANGE instead of LOST_RESET. Since no one is counting
on this event at the moment, no one has software that would try to interpret
LOST_RESET events so we can change it.
I'm thinking its probably more important to be consistent than creating a new
event type. I admit, I didn't follow this whole thread in detail and maybe
there was a good reason to separate out LOST_RESET. By using
audit_do_config_change() you also become consistent with other rules like if
config changes are disallowed because we are immutable.
These changes are on the logging side. This won't affect integration with
auditctl. If you do want to keep LOST_RESET, then it affects all searching and
reporting utilities.
-Steve
> + audit_log_end(ab);
> + return lost;
> + }
>
> break;
>
> }
>
> case AUDIT_GET_FEATURE:
> --
> 1.7.1
>
> --
> Linux-audit mailing list
> Linux-audit(a)redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit