linux-kernel - Re: [PATCH v36 26/33] Audit: Allow multiple records in an audit

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: <CAHC9VhT-qo=N4k7fWvZNELNLTGDWuE4kDXEOYwMBgZGRQu8f7w@mail.gmail.com>
Date:   Fri, 10 Jun 2022 17:04:38 -0400
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,
        stephen.smalley.work@...il.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v36 26/33] Audit: Allow multiple records in an audit_buffer

On Thu, Jun 9, 2022 at 7:15 PM Casey Schaufler <casey@...aufler-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. Functions are created to manage the skb list
> in the audit_buffer.
>
> Suggested-by: Paul Moore <paul@...l-moore.com>
> Signed-off-by: Casey Schaufler <casey@...aufler-ca.com>
> ---
>  kernel/audit.c | 113 +++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 90 insertions(+), 23 deletions(-)

...

> diff --git a/kernel/audit.c b/kernel/audit.c
> index 6b6c089512f7..987740374dfa 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -1784,8 +1789,12 @@ static struct audit_buffer *audit_buffer_alloc(struct audit_context *ctx,
>         ab->skb = nlmsg_new(AUDIT_BUFSIZ, gfp_mask);
>         if (!ab->skb)
>                 goto err;
> +
> +       skb_queue_head_init(&ab->skb_list);
> +       skb_queue_tail(&ab->skb_list, ab->skb);
> +
>         if (!nlmsg_put(ab->skb, 0, 0, type, 0, 0))
> -               goto err;
> +               kfree_skb(ab->skb);
>
>         ab->ctx = ctx;
>         ab->gfp_mask = gfp_mask;

I didn't notice this in v35, but if the nlmsg_put() fails I think you
need to preserve the 'goto err;' since the skb hasn't been properly
initialized.  The good news is that I don't think you need to worry
about the 'kfree_skb(ab->skb);' in the error handler as it's already
been placed on the audit_buffer:skb_list list and will be freed when
audit_buffer_alloc()'s error handling code calls audit_buffer_free().

-- 
paul-moore.com