lkml.org 
[lkml]   [2022]   [Mar]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Date
    SubjectRe: [PATCH v33 25/29] Audit: Allow multiple records in an audit_buffer
    On Thu, Mar 10, 2022 at 6:59 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
    >
    > 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>
    > ---
    > kernel/audit.c | 53 +++++++++++++++++++++++++++++++++-----------------
    > 1 file changed, 35 insertions(+), 18 deletions(-)
    >
    > diff --git a/kernel/audit.c b/kernel/audit.c
    > index f012c3786264..4713e66a12af 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;
    > };
    >
    > @@ -1744,7 +1746,6 @@ static void audit_buffer_free(struct audit_buffer *ab)
    > if (!ab)
    > return;
    >
    > - kfree_skb(ab->skb);

    I like the safety in knowing that audit_buffer_free() would free the
    ab->skb memory, I'm not sure I want to get rid of that. With the
    understanding that ab->skb is always going to be present somewhere in
    ab->skb_list, any reason not to do something like this?

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

    > kmem_cache_free(audit_buffer_cache, ab);
    > }
    >
    > @@ -1760,11 +1761,15 @@ static struct audit_buffer *audit_buffer_alloc(struct audit_context *ctx,
    > ab->skb = nlmsg_new(AUDIT_BUFSIZ, gfp_mask);
    > if (!ab->skb)
    > goto err;
    > - if (!nlmsg_put(ab->skb, 0, 0, type, 0, 0))
    > + if (!nlmsg_put(ab->skb, 0, 0, type, 0, 0)) {
    > + kfree_skb(ab->skb);
    > goto err;
    > + }

    Assuming we restore the audit_buffer_free() functionality as mentioned
    above, if we move the ab->skb_list init and enqueue calls before we
    attempt the nlmsg_put() we can drop the kfree_skb() call and just use
    the existing audit_buffer_free() call at the err target.


    > ab->ctx = ctx;
    > ab->gfp_mask = gfp_mask;
    > + skb_queue_head_init(&ab->skb_list);
    > + skb_queue_tail(&ab->skb_list, ab->skb);
    >
    > return ab;
    >

    --
    paul-moore.com

    \
     
     \ /
      Last update: 2022-03-16 00:48    [W:8.381 / U:1.024 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site