diff mbox series

[v30,24/28] Audit: Add framework for auxiliary records

Message ID 20211124014332.36128-25-casey@schaufler-ca.com (mailing list archive)
State Changes Requested
Delegated to: Paul Moore
Headers show
Series [v30,01/28] integrity: disassociate ima_filter_rule from security_audit_rule | expand

Commit Message

Casey Schaufler Nov. 24, 2021, 1:43 a.m. UTC
Add a list for auxiliary record data to the audit_buffer structure.
Add the audit_stamp information to the audit_buffer as there's no
guarantee that there will be an audit_context containing the stamp
associated with the event. At audit_log_end() time create auxiliary
records (none are currently defined) as have been added to the list.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 kernel/audit.c   | 85 ++++++++++++++++++++++++++++++++++++++++++------
 kernel/audit.h   |  1 +
 kernel/auditsc.c |  2 ++
 3 files changed, 78 insertions(+), 10 deletions(-)

Comments

Paul Moore Dec. 6, 2021, 2:45 a.m. UTC | #1
On Tue, Nov 23, 2021 at 9:10 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> Add a list for auxiliary record data to the audit_buffer structure.
> Add the audit_stamp information to the audit_buffer as there's no
> guarantee that there will be an audit_context containing the stamp
> associated with the event. At audit_log_end() time create auxiliary
> records (none are currently defined) as have been added to the list.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
>  kernel/audit.c   | 85 ++++++++++++++++++++++++++++++++++++++++++------
>  kernel/audit.h   |  1 +
>  kernel/auditsc.c |  2 ++
>  3 files changed, 78 insertions(+), 10 deletions(-)

...

> diff --git a/kernel/audit.c b/kernel/audit.c
> index 069cd4c81a61..2b22498d3532 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -2393,6 +2403,61 @@ void audit_log_end(struct audit_buffer *ab)
>                 wake_up_interruptible(&kauditd_wait);
>         } else
>                 audit_log_lost("rate limit exceeded");
> +}
> +
> +/**
> + * audit_log_end - end one audit record
> + * @ab: the audit_buffer
> + *
> + * Let __audit_log_end() handle the message while the buffer housekeeping
> + * is done here.
> + * If there are other records that have been deferred for the event
> + * create them here.
> + */
> +void audit_log_end(struct audit_buffer *ab)
> +{
> +       struct audit_context_entry *entry;
> +       struct audit_context mcontext;
> +       struct audit_context *mctx;
> +       struct audit_buffer *mab;
> +       struct list_head *l;
> +       struct list_head *n;
> +
> +       if (!ab)
> +               return;
> +
> +       __audit_log_end(ab);
> +
> +       if (list_empty(&ab->aux_records)) {
> +               audit_buffer_free(ab);
> +               return;
> +       }
> +
> +       if (ab->ctx == NULL) {
> +               mcontext.context = AUDIT_CTX_AUXRECORD;

More on this below, but I don't think we need, or want, the line above.

> diff --git a/kernel/audit.h b/kernel/audit.h
> index 56560846f3b0..f87da8e0a5a4 100644
> --- a/kernel/audit.h
> +++ b/kernel/audit.h
> @@ -112,6 +112,7 @@ struct audit_context {
>                 AUDIT_CTX_UNUSED,       /* audit_context is currently unused */
>                 AUDIT_CTX_SYSCALL,      /* in use by syscall */
>                 AUDIT_CTX_URING,        /* in use by io_uring */
> +               AUDIT_CTX_AUXRECORD,    /* in use for auxiliary records */
>         } context;

We shouldn't need to do this here, and I wouldn't recommend this as a
solution even if we were running into problems in audit_log_exit().
The "context" field in the audit_context struct is to identify the
execution context of the task which is generating the audit record(s).

I'm trying to think of a case in this patchset where you would find
"audit_context->context == AUDIT_CTX_AUXRECORD" and I keep coming up
blank, are you certain you hit that case with the code you posted
here?  If so, could you help me understand that situation?

--
paul moore
www.paul-moore.com
diff mbox series

Patch

diff --git a/kernel/audit.c b/kernel/audit.c
index 069cd4c81a61..2b22498d3532 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -191,15 +191,25 @@  static struct audit_ctl_mutex {
  * should be at least that large. */
 #define AUDIT_BUFSIZ 1024
 
+/* The audit_context_entry contains data required to create an
+ * auxiliary record.
+ */
+struct audit_context_entry {
+	struct list_head	list;
+	int			type;	/* Audit record type */
+};
+
 /* The audit_buffer is used when formatting an audit record.  The caller
  * locks briefly to get the record off the freelist or to allocate the
  * buffer, and locks briefly to send the buffer to the netlink layer or
  * to place it on a transmit queue.  Multiple audit_buffers can be in
  * use simultaneously. */
 struct audit_buffer {
-	struct sk_buff       *skb;	/* formatted skb ready to send */
-	struct audit_context *ctx;	/* NULL or associated context */
-	gfp_t		     gfp_mask;
+	struct sk_buff		*skb;	/* formatted skb ready to send */
+	struct audit_context	*ctx;	/* NULL or associated context */
+	struct list_head	aux_records;	/* aux record data */
+	struct audit_stamp	stamp;	/* event stamp */
+	gfp_t			gfp_mask;
 };
 
 struct audit_reply {
@@ -1753,6 +1763,7 @@  static struct audit_buffer *audit_buffer_alloc(struct audit_context *ctx,
 
 	ab->ctx = ctx;
 	ab->gfp_mask = gfp_mask;
+	INIT_LIST_HEAD(&ab->aux_records);
 
 	return ab;
 
@@ -1813,7 +1824,6 @@  struct audit_buffer *audit_log_start(struct audit_context *ctx, gfp_t gfp_mask,
 				     int type)
 {
 	struct audit_buffer *ab;
-	struct audit_stamp stamp;
 
 	if (audit_initialized != AUDIT_INITIALIZED)
 		return NULL;
@@ -1866,14 +1876,14 @@  struct audit_buffer *audit_log_start(struct audit_context *ctx, gfp_t gfp_mask,
 		return NULL;
 	}
 
-	audit_get_stamp(ab->ctx, &stamp);
+	audit_get_stamp(ab->ctx, &ab->stamp);
 	/* cancel dummy context to enable supporting records */
 	if (ctx)
 		ctx->dummy = 0;
 	audit_log_format(ab, "audit(%llu.%03lu:%u): ",
-			 (unsigned long long)stamp.ctime.tv_sec,
-			 stamp.ctime.tv_nsec/1000000,
-			 stamp.serial);
+			 (unsigned long long)ab->stamp.ctime.tv_sec,
+			 ab->stamp.ctime.tv_nsec/1000000,
+			 ab->stamp.serial);
 
 	return ab;
 }
@@ -2363,7 +2373,7 @@  int audit_signal_info(int sig, struct task_struct *t)
 }
 
 /**
- * audit_log_end - end one audit record
+ * __audit_log_end - end one audit record
  * @ab: the audit_buffer
  *
  * We can not do a netlink send inside an irq context because it blocks (last
@@ -2371,7 +2381,7 @@  int audit_signal_info(int sig, struct task_struct *t)
  * queue and a kthread is scheduled to remove them from the queue outside the
  * irq context.  May be called in any context.
  */
-void audit_log_end(struct audit_buffer *ab)
+void __audit_log_end(struct audit_buffer *ab)
 {
 	struct sk_buff *skb;
 	struct nlmsghdr *nlh;
@@ -2393,6 +2403,61 @@  void audit_log_end(struct audit_buffer *ab)
 		wake_up_interruptible(&kauditd_wait);
 	} else
 		audit_log_lost("rate limit exceeded");
+}
+
+/**
+ * audit_log_end - end one audit record
+ * @ab: the audit_buffer
+ *
+ * Let __audit_log_end() handle the message while the buffer housekeeping
+ * is done here.
+ * If there are other records that have been deferred for the event
+ * create them here.
+ */
+void audit_log_end(struct audit_buffer *ab)
+{
+	struct audit_context_entry *entry;
+	struct audit_context mcontext;
+	struct audit_context *mctx;
+	struct audit_buffer *mab;
+	struct list_head *l;
+	struct list_head *n;
+
+	if (!ab)
+		return;
+
+	__audit_log_end(ab);
+
+	if (list_empty(&ab->aux_records)) {
+		audit_buffer_free(ab);
+		return;
+	}
+
+	if (ab->ctx == NULL) {
+		mcontext.context = AUDIT_CTX_AUXRECORD;
+		mcontext.stamp = ab->stamp;
+		mctx = &mcontext;
+	} else
+		mctx = ab->ctx;
+
+	list_for_each_safe(l, n, &ab->aux_records) {
+		entry = list_entry(l, struct audit_context_entry, list);
+		mab = audit_log_start(mctx, ab->gfp_mask, entry->type);
+		if (!mab) {
+			audit_panic("alloc error in audit_log_end");
+			continue;
+		}
+		switch (entry->type) {
+		/* Don't know of any quite yet. */
+		default:
+			audit_panic("Unknown type in audit_log_end");
+			break;
+		}
+		__audit_log_end(mab);
+		audit_buffer_free(mab);
+		list_del(&entry->list);
+		kfree(entry);
+	}
 
 	audit_buffer_free(ab);
 }
diff --git a/kernel/audit.h b/kernel/audit.h
index 56560846f3b0..f87da8e0a5a4 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -112,6 +112,7 @@  struct audit_context {
 		AUDIT_CTX_UNUSED,	/* audit_context is currently unused */
 		AUDIT_CTX_SYSCALL,	/* in use by syscall */
 		AUDIT_CTX_URING,	/* in use by io_uring */
+		AUDIT_CTX_AUXRECORD,	/* in use for auxiliary records */
 	} context;
 	enum audit_state    state, current_state;
 	struct audit_stamp  stamp;	/* event identifier */
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index e6868d072648..c128f7e73e89 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1681,6 +1681,8 @@  static void audit_log_exit(void)
 	case AUDIT_CTX_URING:
 		audit_log_uring(context);
 		break;
+	case AUDIT_CTX_AUXRECORD:
+		break;
 	default:
 		BUG();
 		break;