linux-kernel - Re: [PATCH v32 24/28] Audit: Add framework for auxiliary records

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHC9VhQEJOu_G16eSfMipbMyU-fQZW6bnm2OdstmKHLdgm9Nsg@mail.gmail.com>
Date:   Fri, 4 Mar 2022 09:43:34 -0500
From:   Paul Moore <paul@...l-moore.com>
To:     Casey Schaufler <casey@...aufler-ca.com>
Cc:     casey.schaufler@...el.com, jmorris@...ei.org,
        linux-security-module@...r.kernel.org, selinux@...r.kernel.org,
        linux-audit@...hat.com, keescook@...omium.org,
        john.johansen@...onical.com, penguin-kernel@...ove.sakura.ne.jp,
        sds@...ho.nsa.gov, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v32 24/28] Audit: Add framework for auxiliary records

On Thu, Mar 3, 2022 at 9:13 PM Casey Schaufler <casey@...aufler-ca.com> wrote:
> On 3/3/2022 3:36 PM, Paul Moore wrote:
> > Adding a new aux record would involve calling some private audit
> > function (no one outside of the audit subsystem should need access)
> > that would allocate a new skb similar to what we do in
> > audit_buffer_alloc() and add it to the end of the sk_buff_head list
> > via skb_queue_tail() and resetting audit_buffer::skb to point to the
> > newly allocated skb.
>
> Good naming may be tricky as we need to indicate that a new buffer is
> being allocated for an attached aux record and that the buffer to which
> it's being attached is going to temporarily be in a curious state.
> audit_buffer_add_aux() seems wordy, but it's what I'll start with lacking
> a better suggestion.

I agree that it will leave the audit_buffer in an odd state, at least
with the current definition of the audit_buffer.  However, this is
mitigated by the restriction that the only callers should be within
the audit subsystem.  Here is some quick pseudo-code mockup of what
I'm thinking:

/* on success, ab->skb will point to the new aux record */
static int audit_buffer_aux_new(struct audit_buffer *ab, int type)
{
  WARN_ON(ab->skb != skb_peek(&ab->skb_list));

  ab->skb = nlmsg_new(AUDIT_BUFSIZ, ab->gfp_mask);
  if (!ab->skb)
    goto err;
  if (!nlmsg_put(ab->skb, 0, 0, type, 0, 0))
    goto err;
  skb_queue_tail(&ab->skb_list);
  return 0;

err:
  kfree_skb(&ab->skb);
  ab->skb = skb_peek(&ab->skb_list);
  return -ENOMEM;
}

/* restores the "main" record into ab->skb */
static void audit_buffer_aux_end(struct audit_buffer *ab)
{
  ab->skb = skb_peek(&ab->skb_list);
}

/* free the current aux record and reset ab->skb to the "main" */
static void audit_buffer_aux_cancel(struct audit_buffer *ab)
{
  if (ab->skb != skb_peek_tail(&ab->skb_list)) {
    BUG();
    return;
  }
  ab->skb = skb_peek(&ab->skb_list);
  kfree_skb(skb_dequeue_tail(&ab->skb_list));
}

-- 
paul-moore.com