From: John Johansen <john.johansen@canonical.com> To: Casey Schaufler <casey@schaufler-ca.com>, casey.schaufler@intel.com, jmorris@namei.org, linux-security-module@vger.kernel.org, selinux@vger.kernel.org Cc: linux-audit@redhat.com, keescook@chromium.org, penguin-kernel@i-love.sakura.ne.jp, paul@paul-moore.com, stephen.smalley.work@gmail.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH v35 25/29] Audit: Allow multiple records in an audit_buffer Date: Mon, 25 Apr 2022 18:06:13 -0700 [thread overview] Message-ID: <81c9f88f-7e8f-0ca6-56b8-049571af6809@canonical.com> (raw) In-Reply-To: <20220418145945.38797-26-casey@schaufler-ca.com> On 4/18/22 07:59, Casey Schaufler 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> I agree with Paul that audit_buffer_aux_new() and audit_buffer_aux_end() belong in this patch > --- > 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? > kmem_cache_free(audit_buffer_cache, ab); > } > > @@ -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); why is this no longer an error? > > ab->ctx = ctx; > ab->gfp_mask = gfp_mask; > @@ -1849,7 +1858,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; > @@ -1904,14 +1912,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; > } > @@ -2402,26 +2410,14 @@ int audit_signal_info(int sig, struct task_struct *t) > } > > /** > - * 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 > - * arg, flags, is not set to MSG_DONTWAIT), so the audit buffer is placed on a > - * queue and a kthread is scheduled to remove them from the queue outside the > - * irq context. May be called in any context. > + * __audit_log_end - enqueue one audit record > + * @skb: the buffer to send > */ > -void audit_log_end(struct audit_buffer *ab) > +static void __audit_log_end(struct sk_buff *skb) > { > - struct sk_buff *skb; > struct nlmsghdr *nlh; > > - if (!ab) > - return; > - > if (audit_rate_check()) { > - skb = ab->skb; > - ab->skb = NULL; > - > /* setup the netlink header, see the comments in > * kauditd_send_multicast_skb() for length quirks */ > nlh = nlmsg_hdr(skb); > @@ -2432,6 +2428,26 @@ 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 > + * > + * We can not do a netlink send inside an irq context because it blocks (last > + * arg, flags, is not set to MSG_DONTWAIT), so the audit buffer is placed on a > + * 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) > +{ > + struct sk_buff *skb; > + > + if (!ab) > + return; > + > + while ((skb = skb_dequeue(&ab->skb_list))) > + __audit_log_end(skb); > > audit_buffer_free(ab); > }
WARNING: multiple messages have this Message-ID (diff)
From: John Johansen <john.johansen@canonical.com> To: Casey Schaufler <casey@schaufler-ca.com>, casey.schaufler@intel.com, jmorris@namei.org, linux-security-module@vger.kernel.org, selinux@vger.kernel.org Cc: linux-kernel@vger.kernel.org, linux-audit@redhat.com Subject: Re: [PATCH v35 25/29] Audit: Allow multiple records in an audit_buffer Date: Mon, 25 Apr 2022 18:06:13 -0700 [thread overview] Message-ID: <81c9f88f-7e8f-0ca6-56b8-049571af6809@canonical.com> (raw) In-Reply-To: <20220418145945.38797-26-casey@schaufler-ca.com> On 4/18/22 07:59, Casey Schaufler 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> I agree with Paul that audit_buffer_aux_new() and audit_buffer_aux_end() belong in this patch > --- > 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? > kmem_cache_free(audit_buffer_cache, ab); > } > > @@ -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); why is this no longer an error? > > ab->ctx = ctx; > ab->gfp_mask = gfp_mask; > @@ -1849,7 +1858,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; > @@ -1904,14 +1912,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; > } > @@ -2402,26 +2410,14 @@ int audit_signal_info(int sig, struct task_struct *t) > } > > /** > - * 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 > - * arg, flags, is not set to MSG_DONTWAIT), so the audit buffer is placed on a > - * queue and a kthread is scheduled to remove them from the queue outside the > - * irq context. May be called in any context. > + * __audit_log_end - enqueue one audit record > + * @skb: the buffer to send > */ > -void audit_log_end(struct audit_buffer *ab) > +static void __audit_log_end(struct sk_buff *skb) > { > - struct sk_buff *skb; > struct nlmsghdr *nlh; > > - if (!ab) > - return; > - > if (audit_rate_check()) { > - skb = ab->skb; > - ab->skb = NULL; > - > /* setup the netlink header, see the comments in > * kauditd_send_multicast_skb() for length quirks */ > nlh = nlmsg_hdr(skb); > @@ -2432,6 +2428,26 @@ 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 > + * > + * We can not do a netlink send inside an irq context because it blocks (last > + * arg, flags, is not set to MSG_DONTWAIT), so the audit buffer is placed on a > + * 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) > +{ > + struct sk_buff *skb; > + > + if (!ab) > + return; > + > + while ((skb = skb_dequeue(&ab->skb_list))) > + __audit_log_end(skb); > > audit_buffer_free(ab); > } -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit
next prev parent reply other threads:[~2022-04-26 1:06 UTC|newest] Thread overview: 132+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <20220418145945.38797-1-casey.ref@schaufler-ca.com> 2022-04-18 14:59 ` [PATCH v35 00/29] LSM: Module stacking for AppArmor Casey Schaufler 2022-04-18 14:59 ` Casey Schaufler 2022-04-18 14:59 ` [PATCH v35 01/29] integrity: disassociate ima_filter_rule from security_audit_rule Casey Schaufler 2022-04-18 14:59 ` Casey Schaufler 2022-04-21 16:51 ` John Johansen 2022-04-21 16:51 ` John Johansen 2022-04-18 14:59 ` [PATCH v35 02/29] LSM: Infrastructure management of the sock security Casey Schaufler 2022-04-18 14:59 ` Casey Schaufler 2022-04-18 14:59 ` [PATCH v35 03/29] LSM: Add the lsmblob data structure Casey Schaufler 2022-04-18 14:59 ` Casey Schaufler 2022-04-26 23:15 ` John Johansen 2022-04-26 23:15 ` John Johansen 2022-04-18 14:59 ` [PATCH v35 04/29] LSM: provide lsm name and id slot mappings Casey Schaufler 2022-04-18 14:59 ` Casey Schaufler 2022-04-21 16:50 ` John Johansen 2022-04-21 16:50 ` John Johansen 2022-04-18 14:59 ` [PATCH v35 05/29] IMA: avoid label collisions with stacked LSMs Casey Schaufler 2022-04-18 14:59 ` Casey Schaufler 2022-04-19 16:50 ` Casey Schaufler 2022-04-20 19:23 ` Mimi Zohar 2022-04-20 21:15 ` Casey Schaufler 2022-04-21 3:22 ` Mimi Zohar 2022-04-21 16:50 ` John Johansen 2022-04-21 16:50 ` John Johansen 2022-04-18 14:59 ` [PATCH v35 06/29] LSM: Use lsmblob in security_audit_rule_match Casey Schaufler 2022-04-18 14:59 ` Casey Schaufler 2022-04-21 16:49 ` John Johansen 2022-04-21 16:49 ` John Johansen 2022-04-18 14:59 ` [PATCH v35 07/29] LSM: Use lsmblob in security_kernel_act_as Casey Schaufler 2022-04-18 14:59 ` Casey Schaufler 2022-04-18 14:59 ` [PATCH v35 08/29] LSM: Use lsmblob in security_secctx_to_secid Casey Schaufler 2022-04-18 14:59 ` Casey Schaufler 2022-04-27 0:38 ` John Johansen 2022-04-27 0:38 ` John Johansen 2022-04-18 14:59 ` [PATCH v35 09/29] LSM: Use lsmblob in security_secid_to_secctx Casey Schaufler 2022-04-18 14:59 ` Casey Schaufler 2022-04-18 14:59 ` [PATCH v35 10/29] LSM: Use lsmblob in security_ipc_getsecid Casey Schaufler 2022-04-18 14:59 ` Casey Schaufler 2022-04-18 14:59 ` [PATCH v35 11/29] LSM: Use lsmblob in security_current_getsecid Casey Schaufler 2022-04-18 14:59 ` Casey Schaufler 2022-04-18 14:59 ` [PATCH v35 12/29] LSM: Use lsmblob in security_inode_getsecid Casey Schaufler 2022-04-18 14:59 ` Casey Schaufler 2022-04-18 14:59 ` [PATCH v35 13/29] LSM: Use lsmblob in security_cred_getsecid Casey Schaufler 2022-04-18 14:59 ` Casey Schaufler 2022-04-18 18:02 ` kernel test robot 2022-04-18 18:02 ` kernel test robot 2022-04-19 0:41 ` kernel test robot 2022-04-19 0:41 ` kernel test robot 2022-04-19 0:51 ` kernel test robot 2022-04-19 0:51 ` kernel test robot 2022-04-18 14:59 ` [PATCH v35 14/29] LSM: Specify which LSM to display Casey Schaufler 2022-04-18 14:59 ` Casey Schaufler 2022-04-18 14:59 ` [PATCH v35 15/29] LSM: Ensure the correct LSM context releaser Casey Schaufler 2022-04-18 14:59 ` Casey Schaufler 2022-04-18 14:59 ` [PATCH v35 16/29] LSM: Use lsmcontext in security_secid_to_secctx Casey Schaufler 2022-04-18 14:59 ` Casey Schaufler 2022-04-18 14:59 ` [PATCH v35 17/29] LSM: Use lsmcontext in security_inode_getsecctx Casey Schaufler 2022-04-18 14:59 ` Casey Schaufler 2022-04-18 14:59 ` [PATCH v35 18/29] LSM: security_secid_to_secctx in netlink netfilter Casey Schaufler 2022-04-18 14:59 ` Casey Schaufler 2022-04-18 14:59 ` [PATCH v35 19/29] NET: Store LSM netlabel data in a lsmblob Casey Schaufler 2022-04-18 14:59 ` Casey Schaufler 2022-04-18 14:59 ` [PATCH v35 20/29] binder: Pass LSM identifier for confirmation Casey Schaufler 2022-04-18 14:59 ` Casey Schaufler 2022-04-18 14:59 ` [PATCH v35 21/29] LSM: Extend security_secid_to_secctx to include module selection Casey Schaufler 2022-04-18 14:59 ` Casey Schaufler 2022-04-25 23:32 ` John Johansen 2022-04-25 23:32 ` John Johansen 2022-04-18 14:59 ` [PATCH v35 22/29] Audit: Keep multiple LSM data in audit_names Casey Schaufler 2022-04-18 14:59 ` Casey Schaufler 2022-04-25 23:32 ` John Johansen 2022-04-25 23:32 ` John Johansen 2022-04-26 17:57 ` Paul Moore 2022-04-26 17:57 ` Paul Moore 2022-04-18 14:59 ` [PATCH v35 23/29] Audit: Create audit_stamp structure Casey Schaufler 2022-04-18 14:59 ` Casey Schaufler 2022-04-25 23:31 ` John Johansen 2022-04-25 23:31 ` John Johansen 2022-04-26 18:03 ` Paul Moore 2022-04-26 18:03 ` Paul Moore 2022-04-26 18:58 ` John Johansen 2022-04-26 18:58 ` John Johansen 2022-04-26 19:18 ` Paul Moore 2022-04-26 19:18 ` Paul Moore 2022-04-27 15:49 ` Casey Schaufler 2022-04-27 15:49 ` Casey Schaufler 2022-04-27 16:02 ` Paul Moore 2022-04-27 16:02 ` Paul Moore 2022-04-27 20:55 ` Casey Schaufler 2022-04-27 20:55 ` Casey Schaufler 2022-04-18 14:59 ` [PATCH v35 24/29] LSM: Add a function to report multiple LSMs Casey Schaufler 2022-04-18 14:59 ` Casey Schaufler 2022-04-22 16:26 ` Paul Moore 2022-04-22 16:26 ` Paul Moore 2022-04-25 23:33 ` John Johansen 2022-04-25 23:33 ` John Johansen 2022-04-18 14:59 ` [PATCH v35 25/29] Audit: Allow multiple records in an audit_buffer Casey Schaufler 2022-04-18 14:59 ` Casey Schaufler 2022-04-22 16:27 ` Paul Moore 2022-04-22 16:27 ` Paul Moore 2022-04-26 1:06 ` John Johansen [this message] 2022-04-26 1:06 ` John Johansen 2022-04-26 18:12 ` Paul Moore 2022-04-26 18:12 ` Paul Moore 2022-04-26 19:01 ` John Johansen 2022-04-26 19:01 ` John Johansen 2022-04-18 14:59 ` [PATCH v35 26/29] Audit: Add record for multiple task security contexts Casey Schaufler 2022-04-18 14:59 ` Casey Schaufler 2022-04-22 16:28 ` Paul Moore 2022-04-22 16:28 ` Paul Moore 2022-04-26 1:08 ` John Johansen 2022-04-26 1:08 ` John Johansen 2022-04-26 18:15 ` Paul Moore 2022-04-26 18:15 ` Paul Moore 2022-04-26 19:07 ` John Johansen 2022-04-26 19:07 ` John Johansen 2022-04-18 14:59 ` [PATCH v35 27/29] Audit: Add record for multiple object contexts Casey Schaufler 2022-04-18 14:59 ` Casey Schaufler 2022-04-22 16:29 ` Paul Moore 2022-04-22 16:29 ` Paul Moore 2022-04-26 3:37 ` John Johansen 2022-04-26 3:37 ` John Johansen 2022-04-26 18:57 ` Paul Moore 2022-04-26 18:57 ` Paul Moore 2022-04-26 19:24 ` John Johansen 2022-04-26 19:24 ` John Johansen 2022-04-18 14:59 ` [PATCH v35 28/29] LSM: Add /proc attr entry for full LSM context Casey Schaufler 2022-04-18 14:59 ` Casey Schaufler 2022-04-22 8:37 ` John Johansen 2022-04-22 8:37 ` John Johansen 2022-04-18 14:59 ` [PATCH v35 29/29] AppArmor: Remove the exclusive flag Casey Schaufler 2022-04-18 14:59 ` Casey Schaufler
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=81c9f88f-7e8f-0ca6-56b8-049571af6809@canonical.com \ --to=john.johansen@canonical.com \ --cc=casey.schaufler@intel.com \ --cc=casey@schaufler-ca.com \ --cc=jmorris@namei.org \ --cc=keescook@chromium.org \ --cc=linux-audit@redhat.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-security-module@vger.kernel.org \ --cc=paul@paul-moore.com \ --cc=penguin-kernel@i-love.sakura.ne.jp \ --cc=selinux@vger.kernel.org \ --cc=stephen.smalley.work@gmail.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.