On Mon, Apr 25, 2022 at 9:06 PM John Johansen
[off-list ref] wrote:

On 4/18/22 07:59, Casey Schaufler wrote:

quoted

Replace the single skb pointer in an audit_buffer with
a list of skb pointers. 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.

Suggested-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>

I agree with Paul that audit_buffer_aux_new() and
audit_buffer_aux_end() belong in this patch

quoted

---
 kernel/audit.c | 62 +++++++++++++++++++++++++++++++-------------------
 1 file changed, 39 insertions(+), 23 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index 6b6c089512f7..4d44c05053b0 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c

@@ -197,8 +197,10 @@ static struct audit_ctl_mutex {
  * 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 sk_buff       *skb;      /* the skb for audit_log functions */
+     struct sk_buff_head  skb_list;  /* formatted skbs, ready to send */
      struct audit_context *ctx;      /* NULL or associated context */
+     struct audit_stamp   stamp;     /* audit stamp for these records */
      gfp_t                gfp_mask;
 };

@@ -1765,10 +1767,13 @@ __setup("audit_backlog_limit=", audit_backlog_limit_set);

 static void audit_buffer_free(struct audit_buffer *ab)
 {
+     struct sk_buff *skb;
+
      if (!ab)
              return;

-     kfree_skb(ab->skb);
+     while((skb = skb_dequeue(&ab->skb_list)))
+             kfree_skb(skb);

we still have and ab->skb can we have a debug check that its freed by walking the queue?

By definition ab->skb is always going to point at something on the
list, if it doesn't we are likely to have failures elsewhere.  The
structure definition is private to kernel/audit.c and the
allocation/creation is handled by an allocator function which always
adds the new skb to the list so I think we're okay.

We could add additional checks, but with audit performance already a
hot topic I would prefer to draw the debug-check line at input coming
from outside the audit subsystem.

-- 
paul-moore.com