| Message ID | 20220202235323.23929-25-casey@schaufler-ca.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | [v32,01/28] integrity: disassociate ima_filter_rule from security_audit_rule | expand |
On 2/2/2022 3:53 PM, Casey Schaufler wrote: > Add a list for auxiliary record data to the audit_buffer structure. > 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. > > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> I'm really hoping for either Acks or feedback on this approach. > --- > kernel/audit.c | 84 ++++++++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 74 insertions(+), 10 deletions(-) > > diff --git a/kernel/audit.c b/kernel/audit.c > index f012c3786264..559fb14e0380 100644 > --- a/kernel/audit.c > +++ b/kernel/audit.c > @@ -191,15 +191,25 @@ static struct audit_ctl_mutex { > * should be at least that large. */ > #define AUDIT_BUFSIZ 1024 > > +/* The audit_context_entry contains data required to create an > + * auxiliary record. > + */ > +struct audit_context_entry { > + struct list_head list; > + int type; /* Audit record type */ > +}; > + > /* The audit_buffer is used when formatting an audit record. The caller > * locks briefly to get the record off the freelist or to allocate the > * buffer, and locks briefly to send the buffer to the netlink layer or > * 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 audit_context *ctx; /* NULL or associated context */ > - gfp_t gfp_mask; > + struct sk_buff *skb; /* formatted skb ready to send */ > + struct audit_context *ctx; /* NULL or associated context */ > + struct list_head aux_records; /* aux record data */ > + struct audit_stamp stamp; /* event stamp */ > + gfp_t gfp_mask; > }; > > struct audit_reply { > @@ -1765,6 +1775,7 @@ static struct audit_buffer *audit_buffer_alloc(struct audit_context *ctx, > > ab->ctx = ctx; > ab->gfp_mask = gfp_mask; > + INIT_LIST_HEAD(&ab->aux_records); > > return ab; > > @@ -1825,7 +1836,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; > @@ -1880,14 +1890,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; > } > @@ -2378,7 +2388,7 @@ int audit_signal_info(int sig, struct task_struct *t) > } > > /** > - * audit_log_end - end one audit record > + * __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 > @@ -2386,7 +2396,7 @@ int audit_signal_info(int sig, struct task_struct *t) > * 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) > +void __audit_log_end(struct audit_buffer *ab) > { > struct sk_buff *skb; > struct nlmsghdr *nlh; > @@ -2408,6 +2418,60 @@ 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 > + * > + * Let __audit_log_end() handle the message while the buffer housekeeping > + * is done here. > + * If there are other records that have been deferred for the event > + * create them here. > + */ > +void audit_log_end(struct audit_buffer *ab) > +{ > + struct audit_context_entry *entry; > + struct audit_context mcontext; > + struct audit_context *mctx; > + struct audit_buffer *mab; > + struct list_head *l; > + struct list_head *n; > + > + if (!ab) > + return; > + > + __audit_log_end(ab); > + > + if (list_empty(&ab->aux_records)) { > + audit_buffer_free(ab); > + return; > + } > + > + if (ab->ctx == NULL) { > + mcontext.stamp = ab->stamp; > + mctx = &mcontext; > + } else > + mctx = ab->ctx; > + > + list_for_each_safe(l, n, &ab->aux_records) { > + entry = list_entry(l, struct audit_context_entry, list); > + mab = audit_log_start(mctx, ab->gfp_mask, entry->type); > + if (!mab) { > + audit_panic("alloc error in audit_log_end"); > + continue; > + } > + switch (entry->type) { > + /* Don't know of any quite yet. */ > + default: > + audit_panic("Unknown type in audit_log_end"); > + break; > + } > + __audit_log_end(mab); > + audit_buffer_free(mab); > + list_del(&entry->list); > + kfree(entry); > + } > > audit_buffer_free(ab); > }
On Wed, Mar 2, 2022 at 5:32 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > On 2/2/2022 3:53 PM, Casey Schaufler wrote: > > Add a list for auxiliary record data to the audit_buffer structure. > > 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. > > > > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > > I'm really hoping for either Acks or feedback on this approach. The only callers that make use of this functionality in this patchset is in kernel/audit*.c in patches 25/28 and 26/28, yes?
On 3/3/2022 2:27 PM, Paul Moore wrote: > On Wed, Mar 2, 2022 at 5:32 PM Casey Schaufler <casey@schaufler-ca.com> wrote: >> On 2/2/2022 3:53 PM, Casey Schaufler wrote: >>> Add a list for auxiliary record data to the audit_buffer structure. >>> 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. >>> >>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> >> I'm really hoping for either Acks or feedback on this approach. > The only callers that make use of this functionality in this patchset > is in kernel/audit*.c in patches 25/28 and 26/28, yes? Yes. I think that the container ID record could use it as well. I haven't looked deeply, but it should be usable for any aux record type.
On Thu, Mar 3, 2022 at 5:33 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > On 3/3/2022 2:27 PM, Paul Moore wrote: > > On Wed, Mar 2, 2022 at 5:32 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > >> On 2/2/2022 3:53 PM, Casey Schaufler wrote: > >>> Add a list for auxiliary record data to the audit_buffer structure. > >>> 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. > >>> > >>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > >> I'm really hoping for either Acks or feedback on this approach. > > The only callers that make use of this functionality in this patchset > > is in kernel/audit*.c in patches 25/28 and 26/28, yes? > > Yes. Thanks. I just wanted to make sure you weren't planning on any additional callers in a future revision. I understand that things may change, but I just wanted to make sure there wasn't already something pending. > I think that the container ID record could use it as well. > I haven't looked deeply, but it should be usable for any aux record type. Possibly, but I'm intentionally trying to keep that separated at this stage as the ordering is uncertain. If/when both bits of functionality land we can reconcile things as needed; it's all internal implementation details so we don't have to worry too much about changing it later.
On 3/3/2022 2:43 PM, Paul Moore wrote: > On Thu, Mar 3, 2022 at 5:33 PM Casey Schaufler <casey@schaufler-ca.com> wrote: >> On 3/3/2022 2:27 PM, Paul Moore wrote: >>> On Wed, Mar 2, 2022 at 5:32 PM Casey Schaufler <casey@schaufler-ca.com> wrote: >>>> On 2/2/2022 3:53 PM, Casey Schaufler wrote: >>>>> Add a list for auxiliary record data to the audit_buffer structure. >>>>> 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. >>>>> >>>>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> >>>> I'm really hoping for either Acks or feedback on this approach. >>> The only callers that make use of this functionality in this patchset >>> is in kernel/audit*.c in patches 25/28 and 26/28, yes? >> Yes. > Thanks. I just wanted to make sure you weren't planning on any > additional callers in a future revision. I understand that things may > change, but I just wanted to make sure there wasn't already something > pending. I don't have anything I know about. It's possible that something could be needed when the stacking changes for networking come in, but that's not going to come in for "some time" yet. >> I think that the container ID record could use it as well. >> I haven't looked deeply, but it should be usable for any aux record type. > Possibly, but I'm intentionally trying to keep that separated at this > stage as the ordering is uncertain. If/when both bits of > functionality land we can reconcile things as needed; it's all > internal implementation details so we don't have to worry too much > about changing it later. Agreed, although I'd hate to duplicate mechanism if someone else has an equally functional proposal.
On Wed, Feb 2, 2022 at 7:20 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > > Add a list for auxiliary record data to the audit_buffer structure. > 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. > > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > --- > kernel/audit.c | 84 ++++++++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 74 insertions(+), 10 deletions(-) > > diff --git a/kernel/audit.c b/kernel/audit.c > index f012c3786264..559fb14e0380 100644 > --- a/kernel/audit.c > +++ b/kernel/audit.c > @@ -191,15 +191,25 @@ static struct audit_ctl_mutex { > * should be at least that large. */ > #define AUDIT_BUFSIZ 1024 > > +/* The audit_context_entry contains data required to create an > + * auxiliary record. > + */ > +struct audit_context_entry { > + struct list_head list; > + int type; /* Audit record type */ > +}; Looking at how this ends up being used later in the patchset I think we would be better off if we stored a fully formed audit_buffer in the struct above instead of data fields which we would use to generate an audit_buffer in audit_log_end(). This helps tie the buffer generation logic in with the existing code with which it is most closely related, it allows us to report errors back to the caller as audit_log_end() doesn't historically return an error code, and it helps us get ahead of any future data lifetime issues we might run into by storing the data in this audit struct. This would also simplify things with respect to the audit_buffer struct. Instead of having a dedicated struct for the aux data, you could simply leverage the existing sk_buff list mechanisms: struct audit_buffer { struct sk_buff *skb; /* part of @skb_list, kept for audit_log funcs */ struct sk_buff_head skb_list; struct audit_context *ctx; struct audit_stamp stamp; gfp_t gfp_mask; } The only sneaky bit in the struct above is that we likely want to preserve audit_buffer::skb as a dedicated skb pointer so we don't have to modify all of the audit_log_*() functions; you could of course, but I'm guessing there is little appetite for that in the context of this patchset. 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. This would allow all of the existing audit_log*() functions to work correctly, and when you are done you can restore the "main" skb with skb_peek(). If for some reason you need to fail the new aux record mid-creation you just dequeue the list tail, free the skb, and skb_peek() the "main" skb back into place. > /* The audit_buffer is used when formatting an audit record. The caller > * locks briefly to get the record off the freelist or to allocate the > * buffer, and locks briefly to send the buffer to the netlink layer or > * 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 audit_context *ctx; /* NULL or associated context */ > - gfp_t gfp_mask; > + struct sk_buff *skb; /* formatted skb ready to send */ > + struct audit_context *ctx; /* NULL or associated context */ > + struct list_head aux_records; /* aux record data */ > + struct audit_stamp stamp; /* event stamp */ > + gfp_t gfp_mask; > }; ... > @@ -2408,6 +2418,60 @@ 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 > + * > + * Let __audit_log_end() handle the message while the buffer housekeeping > + * is done here. > + * If there are other records that have been deferred for the event > + * create them here. > + */ > +void audit_log_end(struct audit_buffer *ab) > +{ > + struct audit_context_entry *entry; > + struct audit_context mcontext; > + struct audit_context *mctx; > + struct audit_buffer *mab; > + struct list_head *l; > + struct list_head *n; > + > + if (!ab) > + return; > + > + __audit_log_end(ab); > + > + if (list_empty(&ab->aux_records)) { > + audit_buffer_free(ab); > + return; > + } > + > + if (ab->ctx == NULL) { > + mcontext.stamp = ab->stamp; > + mctx = &mcontext; > + } else > + mctx = ab->ctx; > + > + list_for_each_safe(l, n, &ab->aux_records) { > + entry = list_entry(l, struct audit_context_entry, list); > + mab = audit_log_start(mctx, ab->gfp_mask, entry->type); > + if (!mab) { > + audit_panic("alloc error in audit_log_end"); > + continue; > + } > + switch (entry->type) { > + /* Don't know of any quite yet. */ > + default: > + audit_panic("Unknown type in audit_log_end"); > + break; > + } > + __audit_log_end(mab); > + audit_buffer_free(mab); > + list_del(&entry->list); > + kfree(entry); > + } > > audit_buffer_free(ab); > } This would also allow you to simplify audit_log_end() greatly, I'm sure I'm missing a detail or two, but I suspect it would end up looking something like this: void __audit_log_end(skb) { /* ... current audit_log_end() but with only the sk_buff ... */ } void audit_log_end(ab) { if (!ab) return; while ((skb = skb_dequeue(ab->skb_list))) __audit_log_end(skb); audit_buffer_free(ab); } -- paul-moore.com
On 3/3/2022 3:36 PM, Paul Moore wrote: > On Wed, Feb 2, 2022 at 7:20 PM Casey Schaufler <casey@schaufler-ca.com> wrote: >> Add a list for auxiliary record data to the audit_buffer structure. >> 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. >> >> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> >> --- >> kernel/audit.c | 84 ++++++++++++++++++++++++++++++++++++++++++++------ >> 1 file changed, 74 insertions(+), 10 deletions(-) >> >> diff --git a/kernel/audit.c b/kernel/audit.c >> index f012c3786264..559fb14e0380 100644 >> --- a/kernel/audit.c >> +++ b/kernel/audit.c >> @@ -191,15 +191,25 @@ static struct audit_ctl_mutex { >> * should be at least that large. */ >> #define AUDIT_BUFSIZ 1024 >> >> +/* The audit_context_entry contains data required to create an >> + * auxiliary record. >> + */ >> +struct audit_context_entry { >> + struct list_head list; >> + int type; /* Audit record type */ >> +}; > Looking at how this ends up being used later in the patchset I think > we would be better off if we stored a fully formed audit_buffer in the > struct above instead of data fields which we would use to generate an > audit_buffer in audit_log_end(). This helps tie the buffer generation > logic in with the existing code with which it is most closely related, > it allows us to report errors back to the caller as audit_log_end() > doesn't historically return an error code, and it helps us get ahead > of any future data lifetime issues we might run into by storing the > data in this audit struct. OK, I'll buy that. > This would also simplify things with respect to the audit_buffer > struct. Instead of having a dedicated struct for the aux data, you > could simply leverage the existing sk_buff list mechanisms: I can't say that "simply" is the adverb I'd choose, but sure, I can do this. > struct audit_buffer { > struct sk_buff *skb; /* part of @skb_list, kept for audit_log funcs */ > struct sk_buff_head skb_list; > struct audit_context *ctx; > struct audit_stamp stamp; > gfp_t gfp_mask; > } > > The only sneaky bit in the struct above is that we likely want to > preserve audit_buffer::skb as a dedicated skb pointer so we don't have > to modify all of the audit_log_*() functions; you could of course, but > I'm guessing there is little appetite for that in the context of this > patchset. I will give it a go without making the massive interface change. > 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. > This would allow all of the existing > audit_log*() functions to work correctly, and when you are done you > can restore the "main" skb with skb_peek(). audit_buffer_close_aux() > If for some reason you > need to fail the new aux record mid-creation you just dequeue the list > tail, free the skb, and skb_peek() the "main" skb back into place. Why do I always get nervous when I hear "just" and "skb" in the same sentence? >> /* The audit_buffer is used when formatting an audit record. The caller >> * locks briefly to get the record off the freelist or to allocate the >> * buffer, and locks briefly to send the buffer to the netlink layer or >> * 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 audit_context *ctx; /* NULL or associated context */ >> - gfp_t gfp_mask; >> + struct sk_buff *skb; /* formatted skb ready to send */ >> + struct audit_context *ctx; /* NULL or associated context */ >> + struct list_head aux_records; /* aux record data */ >> + struct audit_stamp stamp; /* event stamp */ >> + gfp_t gfp_mask; >> }; > ... > >> @@ -2408,6 +2418,60 @@ 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 >> + * >> + * Let __audit_log_end() handle the message while the buffer housekeeping >> + * is done here. >> + * If there are other records that have been deferred for the event >> + * create them here. >> + */ >> +void audit_log_end(struct audit_buffer *ab) >> +{ >> + struct audit_context_entry *entry; >> + struct audit_context mcontext; >> + struct audit_context *mctx; >> + struct audit_buffer *mab; >> + struct list_head *l; >> + struct list_head *n; >> + >> + if (!ab) >> + return; >> + >> + __audit_log_end(ab); >> + >> + if (list_empty(&ab->aux_records)) { >> + audit_buffer_free(ab); >> + return; >> + } >> + >> + if (ab->ctx == NULL) { >> + mcontext.stamp = ab->stamp; >> + mctx = &mcontext; >> + } else >> + mctx = ab->ctx; >> + >> + list_for_each_safe(l, n, &ab->aux_records) { >> + entry = list_entry(l, struct audit_context_entry, list); >> + mab = audit_log_start(mctx, ab->gfp_mask, entry->type); >> + if (!mab) { >> + audit_panic("alloc error in audit_log_end"); >> + continue; >> + } >> + switch (entry->type) { >> + /* Don't know of any quite yet. */ >> + default: >> + audit_panic("Unknown type in audit_log_end"); >> + break; >> + } >> + __audit_log_end(mab); >> + audit_buffer_free(mab); >> + list_del(&entry->list); >> + kfree(entry); >> + } >> >> audit_buffer_free(ab); >> } > This would also allow you to simplify audit_log_end() greatly, I'm > sure I'm missing a detail or two, but I suspect it would end up > looking something like this: Agreed. That is a much better fit for the existing code flow. > > void __audit_log_end(skb) > { > /* ... current audit_log_end() but with only the sk_buff ... */ > } > > void audit_log_end(ab) > { > if (!ab) > return; > while ((skb = skb_dequeue(ab->skb_list))) > __audit_log_end(skb); > audit_buffer_free(ab); > } > > > -- > paul-moore.com
On Thu, Mar 3, 2022 at 9:13 PM Casey Schaufler <casey@schaufler-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)); }
diff --git a/kernel/audit.c b/kernel/audit.c index f012c3786264..559fb14e0380 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -191,15 +191,25 @@ static struct audit_ctl_mutex { * should be at least that large. */ #define AUDIT_BUFSIZ 1024 +/* The audit_context_entry contains data required to create an + * auxiliary record. + */ +struct audit_context_entry { + struct list_head list; + int type; /* Audit record type */ +}; + /* The audit_buffer is used when formatting an audit record. The caller * locks briefly to get the record off the freelist or to allocate the * buffer, and locks briefly to send the buffer to the netlink layer or * 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 audit_context *ctx; /* NULL or associated context */ - gfp_t gfp_mask; + struct sk_buff *skb; /* formatted skb ready to send */ + struct audit_context *ctx; /* NULL or associated context */ + struct list_head aux_records; /* aux record data */ + struct audit_stamp stamp; /* event stamp */ + gfp_t gfp_mask; }; struct audit_reply { @@ -1765,6 +1775,7 @@ static struct audit_buffer *audit_buffer_alloc(struct audit_context *ctx, ab->ctx = ctx; ab->gfp_mask = gfp_mask; + INIT_LIST_HEAD(&ab->aux_records); return ab; @@ -1825,7 +1836,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; @@ -1880,14 +1890,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; } @@ -2378,7 +2388,7 @@ int audit_signal_info(int sig, struct task_struct *t) } /** - * audit_log_end - end one audit record + * __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 @@ -2386,7 +2396,7 @@ int audit_signal_info(int sig, struct task_struct *t) * 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) +void __audit_log_end(struct audit_buffer *ab) { struct sk_buff *skb; struct nlmsghdr *nlh; @@ -2408,6 +2418,60 @@ 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 + * + * Let __audit_log_end() handle the message while the buffer housekeeping + * is done here. + * If there are other records that have been deferred for the event + * create them here. + */ +void audit_log_end(struct audit_buffer *ab) +{ + struct audit_context_entry *entry; + struct audit_context mcontext; + struct audit_context *mctx; + struct audit_buffer *mab; + struct list_head *l; + struct list_head *n; + + if (!ab) + return; + + __audit_log_end(ab); + + if (list_empty(&ab->aux_records)) { + audit_buffer_free(ab); + return; + } + + if (ab->ctx == NULL) { + mcontext.stamp = ab->stamp; + mctx = &mcontext; + } else + mctx = ab->ctx; + + list_for_each_safe(l, n, &ab->aux_records) { + entry = list_entry(l, struct audit_context_entry, list); + mab = audit_log_start(mctx, ab->gfp_mask, entry->type); + if (!mab) { + audit_panic("alloc error in audit_log_end"); + continue; + } + switch (entry->type) { + /* Don't know of any quite yet. */ + default: + audit_panic("Unknown type in audit_log_end"); + break; + } + __audit_log_end(mab); + audit_buffer_free(mab); + list_del(&entry->list); + kfree(entry); + } audit_buffer_free(ab); }
Add a list for auxiliary record data to the audit_buffer structure. 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. Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> --- kernel/audit.c | 84 ++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 74 insertions(+), 10 deletions(-)