* [RFC PATCH v1] audit: log AUDIT_TIME_* records only from rules @ 2021-11-04 20:59 Richard Guy Briggs 2021-11-04 21:29 ` Paul Moore 2021-12-29 22:51 ` Paul Moore 0 siblings, 2 replies; 14+ messages in thread From: Richard Guy Briggs @ 2021-11-04 20:59 UTC (permalink / raw) To: Linux-Audit Mailing List; +Cc: Richard Guy Briggs, Eric Paris AUDIT_TIME_* events are generated when there are syscall rules present that are not related to time keeping. This will produce noisy log entries that could flood the logs and hide events we really care about. Rather than immediately produce the AUDIT_TIME_* records, store the data and log it at syscall exit time respecting the filter rules. Please see https://bugzilla.redhat.com/show_bug.cgi?id=1991919 Signed-off-by: Richard Guy Briggs <rgb@redhat.com> --- Note: This is a quick and dirty proof-of-concept. If this approach of storing the values in the audit_context for later filtering is acceptable I'll clean up the patch (re-name functions) and re-submit. kernel/audit.h | 6 ++++++ kernel/auditsc.c | 29 +++++++++++++++++++++++++---- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/kernel/audit.h b/kernel/audit.h index 3b64a97f6091..25d63731b0e0 100644 --- a/kernel/audit.h +++ b/kernel/audit.h @@ -196,6 +196,12 @@ struct audit_context { struct { char *name; } module; + struct { + struct audit_ntp_data data; + } ntp; + struct { + struct timespec64 injoffset; + } tk; }; int fds[2]; struct audit_proctitle proctitle; diff --git a/kernel/auditsc.c b/kernel/auditsc.c index 6efb0bb909d0..8983790ad86a 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -1210,11 +1210,18 @@ static void audit_log_fcaps(struct audit_buffer *ab, struct audit_names *name) from_kuid(&init_user_ns, name->fcap.rootid)); } +void __audit_ntp_log_(const struct audit_ntp_data *ad); + static void show_special(struct audit_context *context, int *call_panic) { struct audit_buffer *ab; int i; + if (context->type == AUDIT_TIME_ADJNTPVAL) { + __audit_ntp_log_(&context->ntp.data); + return; + } + ab = audit_log_start(context, GFP_KERNEL, context->type); if (!ab) return; @@ -1324,6 +1331,11 @@ static void show_special(struct audit_context *context, int *call_panic) audit_log_format(ab, "(null)"); break; + case AUDIT_TIME_INJOFFSET: + audit_log_format(ab, "sec=%lli nsec=%li", + (long long)context->tk.injoffset.tv_sec, + context->tk.injoffset.tv_nsec); + break; } audit_log_end(ab); } @@ -2571,9 +2583,18 @@ void __audit_fanotify(unsigned int response) void __audit_tk_injoffset(struct timespec64 offset) { - audit_log(audit_context(), GFP_KERNEL, AUDIT_TIME_INJOFFSET, - "sec=%lli nsec=%li", - (long long)offset.tv_sec, offset.tv_nsec); + struct audit_context *context = audit_context(); + + context->type = AUDIT_TIME_INJOFFSET; + memcpy(&context->tk.injoffset, &offset, sizeof(offset)); +} + +void __audit_ntp_log(const struct audit_ntp_data *ad) +{ + struct audit_context *context = audit_context(); + + context->type = AUDIT_TIME_ADJNTPVAL; + memcpy(&context->ntp.data, ad, sizeof(*ad)); } static void audit_log_ntp_val(const struct audit_ntp_data *ad, @@ -2588,7 +2609,7 @@ static void audit_log_ntp_val(const struct audit_ntp_data *ad, "op=%s old=%lli new=%lli", op, val->oldval, val->newval); } -void __audit_ntp_log(const struct audit_ntp_data *ad) +void __audit_ntp_log_(const struct audit_ntp_data *ad) { audit_log_ntp_val(ad, "offset", AUDIT_NTP_OFFSET); audit_log_ntp_val(ad, "freq", AUDIT_NTP_FREQ); -- 2.27.0 -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [RFC PATCH v1] audit: log AUDIT_TIME_* records only from rules 2021-11-04 20:59 [RFC PATCH v1] audit: log AUDIT_TIME_* records only from rules Richard Guy Briggs @ 2021-11-04 21:29 ` Paul Moore 2021-11-04 21:53 ` Richard Guy Briggs 2021-12-29 22:51 ` Paul Moore 1 sibling, 1 reply; 14+ messages in thread From: Paul Moore @ 2021-11-04 21:29 UTC (permalink / raw) To: Richard Guy Briggs; +Cc: Eric Paris, Linux-Audit Mailing List On Thu, Nov 4, 2021 at 5:00 PM Richard Guy Briggs <rgb@redhat.com> wrote: > > AUDIT_TIME_* events are generated when there are syscall rules present that are > not related to time keeping. This will produce noisy log entries that could > flood the logs and hide events we really care about. > > Rather than immediately produce the AUDIT_TIME_* records, store the data and > log it at syscall exit time respecting the filter rules. > > Please see https://bugzilla.redhat.com/show_bug.cgi?id=1991919 Unfortunately that URL isn't publicly accessible. It might be helpful to simply add the relevant information to the commit description[1] and omit the link entirely. Since this is just an RFC, please don't resend the patch just to include that information, you can simply reply to this thread with the additional info. -- paul moore www.paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH v1] audit: log AUDIT_TIME_* records only from rules 2021-11-04 21:29 ` Paul Moore @ 2021-11-04 21:53 ` Richard Guy Briggs 2021-11-19 16:15 ` Paul Moore 0 siblings, 1 reply; 14+ messages in thread From: Richard Guy Briggs @ 2021-11-04 21:53 UTC (permalink / raw) To: Paul Moore; +Cc: Eric Paris, Linux-Audit Mailing List On 2021-11-04 17:29, Paul Moore wrote: > On Thu, Nov 4, 2021 at 5:00 PM Richard Guy Briggs <rgb@redhat.com> wrote: > > > > AUDIT_TIME_* events are generated when there are syscall rules present that are > > not related to time keeping. This will produce noisy log entries that could > > flood the logs and hide events we really care about. > > > > Rather than immediately produce the AUDIT_TIME_* records, store the data and > > log it at syscall exit time respecting the filter rules. > > > > Please see https://bugzilla.redhat.com/show_bug.cgi?id=1991919 > > Unfortunately that URL isn't publicly accessible. It might be helpful > to simply add the relevant information to the commit description[1] > and omit the link entirely. Since this is just an RFC, please don't > resend the patch just to include that information, you can simply > reply to this thread with the additional info. Hmmm, sorry about that. There isn't really anything in that bz that shouldn't be public, but I'll check before openning it up... Basically it was a report that: TIME_ADJNTPVAL audit events are not generated if there are no syscalls rules, but that these events are generated when at least one unrelated syscall rule is added. This behaviour was confirmed but the conclusion about what should be the correct behaviour differed from that of the reporter. > paul moore - RGB -- Richard Guy Briggs <rgb@redhat.com> Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada IRC: rgb, SunRaycer Voice: +1.647.777.2635, Internal: (81) 32635 -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH v1] audit: log AUDIT_TIME_* records only from rules 2021-11-04 21:53 ` Richard Guy Briggs @ 2021-11-19 16:15 ` Paul Moore 2021-11-19 18:01 ` Richard Guy Briggs 0 siblings, 1 reply; 14+ messages in thread From: Paul Moore @ 2021-11-19 16:15 UTC (permalink / raw) To: Richard Guy Briggs; +Cc: Eric Paris, Linux-Audit Mailing List On Thu, Nov 4, 2021 at 5:53 PM Richard Guy Briggs <rgb@redhat.com> wrote: > On 2021-11-04 17:29, Paul Moore wrote: > > On Thu, Nov 4, 2021 at 5:00 PM Richard Guy Briggs <rgb@redhat.com> wrote: > > > > > > AUDIT_TIME_* events are generated when there are syscall rules present that are > > > not related to time keeping. This will produce noisy log entries that could > > > flood the logs and hide events we really care about. > > > > > > Rather than immediately produce the AUDIT_TIME_* records, store the data and > > > log it at syscall exit time respecting the filter rules. > > > > > > Please see https://bugzilla.redhat.com/show_bug.cgi?id=1991919 > > > > Unfortunately that URL isn't publicly accessible. It might be helpful > > to simply add the relevant information to the commit description[1] > > and omit the link entirely. Since this is just an RFC, please don't > > resend the patch just to include that information, you can simply > > reply to this thread with the additional info. > > Hmmm, sorry about that. There isn't really anything in that bz that > shouldn't be public, but I'll check before openning it up... > > Basically it was a report that: > TIME_ADJNTPVAL audit events are not generated if there are no syscalls > rules, but that these events are generated when at least one unrelated > syscall rule is added. > > This behaviour was confirmed but the conclusion about what should be the > correct behaviour differed from that of the reporter. I'm still wondering about the best way to handle this situation, and I want to make sure I'm understanding the problem correctly. So I'm clear on the problem, is the issue that the AUDIT_TIME records are being generated whenever at least one syscall filter is present, regardless of if that syscall is time related? With the expected behavior being that AUDIT_TIME records are only generated when a time related syscall is being audited? -- paul moore www.paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH v1] audit: log AUDIT_TIME_* records only from rules 2021-11-19 16:15 ` Paul Moore @ 2021-11-19 18:01 ` Richard Guy Briggs 2021-11-24 15:44 ` Paul Moore 0 siblings, 1 reply; 14+ messages in thread From: Richard Guy Briggs @ 2021-11-19 18:01 UTC (permalink / raw) To: Paul Moore; +Cc: Eric Paris, Linux-Audit Mailing List On 2021-11-19 11:15, Paul Moore wrote: > On Thu, Nov 4, 2021 at 5:53 PM Richard Guy Briggs <rgb@redhat.com> wrote: > > On 2021-11-04 17:29, Paul Moore wrote: > > > On Thu, Nov 4, 2021 at 5:00 PM Richard Guy Briggs <rgb@redhat.com> wrote: > > > > > > > > AUDIT_TIME_* events are generated when there are syscall rules present that are > > > > not related to time keeping. This will produce noisy log entries that could > > > > flood the logs and hide events we really care about. > > > > > > > > Rather than immediately produce the AUDIT_TIME_* records, store the data and > > > > log it at syscall exit time respecting the filter rules. > > > > > > > > Please see https://bugzilla.redhat.com/show_bug.cgi?id=1991919 > > > > > > Unfortunately that URL isn't publicly accessible. It might be helpful > > > to simply add the relevant information to the commit description[1] > > > and omit the link entirely. Since this is just an RFC, please don't > > > resend the patch just to include that information, you can simply > > > reply to this thread with the additional info. > > > > Hmmm, sorry about that. There isn't really anything in that bz that > > shouldn't be public, but I'll check before openning it up... > > > > Basically it was a report that: > > TIME_ADJNTPVAL audit events are not generated if there are no syscalls > > rules, but that these events are generated when at least one unrelated > > syscall rule is added. > > > > This behaviour was confirmed but the conclusion about what should be the > > correct behaviour differed from that of the reporter. > > I'm still wondering about the best way to handle this situation, and I > want to make sure I'm understanding the problem correctly. So I'm > clear on the problem, is the issue that the AUDIT_TIME records are > being generated whenever at least one syscall filter is present, > regardless of if that syscall is time related? With the expected > behavior being that AUDIT_TIME records are only generated when a time > related syscall is being audited? Exactly. > paul moore - RGB -- Richard Guy Briggs <rgb@redhat.com> Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada IRC: rgb, SunRaycer Voice: +1.647.777.2635, Internal: (81) 32635 -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH v1] audit: log AUDIT_TIME_* records only from rules 2021-11-19 18:01 ` Richard Guy Briggs @ 2021-11-24 15:44 ` Paul Moore 2021-12-21 20:50 ` Richard Guy Briggs 0 siblings, 1 reply; 14+ messages in thread From: Paul Moore @ 2021-11-24 15:44 UTC (permalink / raw) To: Richard Guy Briggs; +Cc: Eric Paris, Linux-Audit Mailing List On Fri, Nov 19, 2021 at 1:02 PM Richard Guy Briggs <rgb@redhat.com> wrote: > On 2021-11-19 11:15, Paul Moore wrote: > > On Thu, Nov 4, 2021 at 5:53 PM Richard Guy Briggs <rgb@redhat.com> wrote: > > > On 2021-11-04 17:29, Paul Moore wrote: > > > > On Thu, Nov 4, 2021 at 5:00 PM Richard Guy Briggs <rgb@redhat.com> wrote: > > > > > > > > > > AUDIT_TIME_* events are generated when there are syscall rules present that are > > > > > not related to time keeping. This will produce noisy log entries that could > > > > > flood the logs and hide events we really care about. > > > > > > > > > > Rather than immediately produce the AUDIT_TIME_* records, store the data and > > > > > log it at syscall exit time respecting the filter rules. > > > > > > > > > > Please see https://bugzilla.redhat.com/show_bug.cgi?id=1991919 > > > > > > > > Unfortunately that URL isn't publicly accessible. It might be helpful > > > > to simply add the relevant information to the commit description[1] > > > > and omit the link entirely. Since this is just an RFC, please don't > > > > resend the patch just to include that information, you can simply > > > > reply to this thread with the additional info. > > > > > > Hmmm, sorry about that. There isn't really anything in that bz that > > > shouldn't be public, but I'll check before openning it up... > > > > > > Basically it was a report that: > > > TIME_ADJNTPVAL audit events are not generated if there are no syscalls > > > rules, but that these events are generated when at least one unrelated > > > syscall rule is added. > > > > > > This behaviour was confirmed but the conclusion about what should be the > > > correct behaviour differed from that of the reporter. > > > > I'm still wondering about the best way to handle this situation, and I > > want to make sure I'm understanding the problem correctly. So I'm > > clear on the problem, is the issue that the AUDIT_TIME records are > > being generated whenever at least one syscall filter is present, > > regardless of if that syscall is time related? With the expected > > behavior being that AUDIT_TIME records are only generated when a time > > related syscall is being audited? > > Exactly. In that case it seems pretty similar to the PATH record and I would think that handling it in a similar manner would be The Right Thing to do ... which looks like what you are doing. You mentioned that you wanted to do some more work on this patch so I'll hold off further comments until the updated patch is posted. -- paul moore www.paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH v1] audit: log AUDIT_TIME_* records only from rules 2021-11-24 15:44 ` Paul Moore @ 2021-12-21 20:50 ` Richard Guy Briggs 2021-12-23 17:38 ` Paul Moore 0 siblings, 1 reply; 14+ messages in thread From: Richard Guy Briggs @ 2021-12-21 20:50 UTC (permalink / raw) To: Paul Moore; +Cc: Eric Paris, Linux-Audit Mailing List On 2021-11-24 10:44, Paul Moore wrote: > On Fri, Nov 19, 2021 at 1:02 PM Richard Guy Briggs <rgb@redhat.com> wrote: > > On 2021-11-19 11:15, Paul Moore wrote: > > > On Thu, Nov 4, 2021 at 5:53 PM Richard Guy Briggs <rgb@redhat.com> wrote: > > > > On 2021-11-04 17:29, Paul Moore wrote: > > > > > On Thu, Nov 4, 2021 at 5:00 PM Richard Guy Briggs <rgb@redhat.com> wrote: > > > > > > > > > > > > AUDIT_TIME_* events are generated when there are syscall rules present that are > > > > > > not related to time keeping. This will produce noisy log entries that could > > > > > > flood the logs and hide events we really care about. > > > > > > > > > > > > Rather than immediately produce the AUDIT_TIME_* records, store the data and > > > > > > log it at syscall exit time respecting the filter rules. > > > > > > > > > > > > Please see https://bugzilla.redhat.com/show_bug.cgi?id=1991919 > > > > > > > > > > Unfortunately that URL isn't publicly accessible. It might be helpful > > > > > to simply add the relevant information to the commit description[1] > > > > > and omit the link entirely. Since this is just an RFC, please don't > > > > > resend the patch just to include that information, you can simply > > > > > reply to this thread with the additional info. > > > > > > > > Hmmm, sorry about that. There isn't really anything in that bz that > > > > shouldn't be public, but I'll check before openning it up... > > > > > > > > Basically it was a report that: > > > > TIME_ADJNTPVAL audit events are not generated if there are no syscalls > > > > rules, but that these events are generated when at least one unrelated > > > > syscall rule is added. > > > > > > > > This behaviour was confirmed but the conclusion about what should be the > > > > correct behaviour differed from that of the reporter. > > > > > > I'm still wondering about the best way to handle this situation, and I > > > want to make sure I'm understanding the problem correctly. So I'm > > > clear on the problem, is the issue that the AUDIT_TIME records are > > > being generated whenever at least one syscall filter is present, > > > regardless of if that syscall is time related? With the expected > > > behavior being that AUDIT_TIME records are only generated when a time > > > related syscall is being audited? > > > > Exactly. > > In that case it seems pretty similar to the PATH record and I would > think that handling it in a similar manner would be The Right Thing to > do ... which looks like what you are doing. > > You mentioned that you wanted to do some more work on this patch so > I'll hold off further comments until the updated patch is posted. I had a look at this patch and there is no further adjustment needed. The only note is that the AUDIT_TIME_ADJNTPVAL record is printed at the top of show_special() due to the need to potentially allocate multiple records. Do you think this requires a comment in the description or in the code just above the call to __audit_ntp_log_()? If not, please merge it at your convenience. Sorry to have dropped this ball. > paul moore - RGB -- Richard Guy Briggs <rgb@redhat.com> Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada IRC: rgb, SunRaycer Voice: +1.647.777.2635, Internal: (81) 32635 -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH v1] audit: log AUDIT_TIME_* records only from rules 2021-12-21 20:50 ` Richard Guy Briggs @ 2021-12-23 17:38 ` Paul Moore 2021-12-24 14:21 ` Richard Guy Briggs 0 siblings, 1 reply; 14+ messages in thread From: Paul Moore @ 2021-12-23 17:38 UTC (permalink / raw) To: Richard Guy Briggs; +Cc: Eric Paris, Linux-Audit Mailing List On Tue, Dec 21, 2021 at 3:50 PM Richard Guy Briggs <rgb@redhat.com> wrote: > On 2021-11-24 10:44, Paul Moore wrote: > > > You mentioned that you wanted to do some more work on this patch so > > I'll hold off further comments until the updated patch is posted. > > I had a look at this patch and there is no further adjustment needed. > The only note is that the AUDIT_TIME_ADJNTPVAL record is printed at the > top of show_special() due to the need to potentially allocate multiple > records. Do you think this requires a comment in the description or > in the code just above the call to __audit_ntp_log_()? > > If not, please merge it at your convenience. Sorry to have dropped this > ball. No worries, I'll put the patch back on the to-review pile. However, since we are at -rc6 this week with the holidays in full swing it seems like this is something that we should defer merging until after the upcoming merge window closes. Now that this is no longer a RFC, I'll try to take a closer look and offer any additional review feedback next week. -- paul moore www.paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH v1] audit: log AUDIT_TIME_* records only from rules 2021-12-23 17:38 ` Paul Moore @ 2021-12-24 14:21 ` Richard Guy Briggs 2021-12-27 15:07 ` Paul Moore 0 siblings, 1 reply; 14+ messages in thread From: Richard Guy Briggs @ 2021-12-24 14:21 UTC (permalink / raw) To: Paul Moore; +Cc: Eric Paris, Linux-Audit Mailing List On 2021-12-23 12:38, Paul Moore wrote: > On Tue, Dec 21, 2021 at 3:50 PM Richard Guy Briggs <rgb@redhat.com> wrote: > > On 2021-11-24 10:44, Paul Moore wrote: > > > You mentioned that you wanted to do some more work on this patch so > > > I'll hold off further comments until the updated patch is posted. > > > > I had a look at this patch and there is no further adjustment needed. > > The only note is that the AUDIT_TIME_ADJNTPVAL record is printed at the > > top of show_special() due to the need to potentially allocate multiple > > records. Do you think this requires a comment in the description or > > in the code just above the call to __audit_ntp_log_()? > > > > If not, please merge it at your convenience. Sorry to have dropped this > > ball. > > No worries, I'll put the patch back on the to-review pile. However, > since we are at -rc6 this week with the holidays in full swing it > seems like this is something that we should defer merging until after > the upcoming merge window closes. Now that this is no longer a RFC, > I'll try to take a closer look and offer any additional review > feedback next week. Darn, missed another merge window. My fault for dropping the ball. If I might advocate, it is a bugfix rather than a feature... > paul moore - RGB -- Richard Guy Briggs <rgb@redhat.com> Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada IRC: rgb, SunRaycer Voice: +1.647.777.2635, Internal: (81) 32635 -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH v1] audit: log AUDIT_TIME_* records only from rules 2021-12-24 14:21 ` Richard Guy Briggs @ 2021-12-27 15:07 ` Paul Moore 0 siblings, 0 replies; 14+ messages in thread From: Paul Moore @ 2021-12-27 15:07 UTC (permalink / raw) To: Richard Guy Briggs; +Cc: Eric Paris, Linux-Audit Mailing List On Fri, Dec 24, 2021 at 9:21 AM Richard Guy Briggs <rgb@redhat.com> wrote: > On 2021-12-23 12:38, Paul Moore wrote: > > On Tue, Dec 21, 2021 at 3:50 PM Richard Guy Briggs <rgb@redhat.com> wrote: > > > On 2021-11-24 10:44, Paul Moore wrote: > > > > You mentioned that you wanted to do some more work on this patch so > > > > I'll hold off further comments until the updated patch is posted. > > > > > > I had a look at this patch and there is no further adjustment needed. > > > The only note is that the AUDIT_TIME_ADJNTPVAL record is printed at the > > > top of show_special() due to the need to potentially allocate multiple > > > records. Do you think this requires a comment in the description or > > > in the code just above the call to __audit_ntp_log_()? > > > > > > If not, please merge it at your convenience. Sorry to have dropped this > > > ball. > > > > No worries, I'll put the patch back on the to-review pile. However, > > since we are at -rc6 this week with the holidays in full swing it > > seems like this is something that we should defer merging until after > > the upcoming merge window closes. Now that this is no longer a RFC, > > I'll try to take a closer look and offer any additional review > > feedback next week. > > Darn, missed another merge window. My fault for dropping the ball. I know you are well aware of this policy Richard, but for others who may be watching this, I don't like to take significant changes into -next much past the -rc5/-rc6 boundary unless it is a critical bug fix. While this particular patch might not be very large, it is a user visible change which makes it a "significant change" in my opinion. https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/audit.git/tree/README.md > If I might advocate, it is a bugfix rather than a feature... Maybe. I can see an argument for viewing it that way, but I also see it as a user visible change. Taking into account where we are at in the current -rcX cycle, the fact that large portions of the kernel development community are in the midst of various end-of-year holidays, and the nature of the patch, I feel it is better to give this more than a week or two in -next. I also feel that if this was truly that important, we would have seen more urgency and comments associated with the patch. -- paul moore www.paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH v1] audit: log AUDIT_TIME_* records only from rules 2021-11-04 20:59 [RFC PATCH v1] audit: log AUDIT_TIME_* records only from rules Richard Guy Briggs 2021-11-04 21:29 ` Paul Moore @ 2021-12-29 22:51 ` Paul Moore 2022-01-12 21:06 ` Richard Guy Briggs 1 sibling, 1 reply; 14+ messages in thread From: Paul Moore @ 2021-12-29 22:51 UTC (permalink / raw) To: Richard Guy Briggs; +Cc: Eric Paris, Linux-Audit Mailing List On Thu, Nov 4, 2021 at 5:00 PM Richard Guy Briggs <rgb@redhat.com> wrote: > > AUDIT_TIME_* events are generated when there are syscall rules present that are > not related to time keeping. This will produce noisy log entries that could > flood the logs and hide events we really care about. > > Rather than immediately produce the AUDIT_TIME_* records, store the data and > log it at syscall exit time respecting the filter rules. > > Please see https://bugzilla.redhat.com/show_bug.cgi?id=1991919 > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > --- > Note: This is a quick and dirty proof-of-concept. If this approach of > storing the values in the audit_context for later filtering is > acceptable I'll clean up the patch (re-name functions) and re-submit. > > kernel/audit.h | 6 ++++++ > kernel/auditsc.c | 29 +++++++++++++++++++++++++---- > 2 files changed, 31 insertions(+), 4 deletions(-) Reviewing this now with a more critical eye since it is longer just a quick-n-dirty proof of concept ... > diff --git a/kernel/audit.h b/kernel/audit.h > index 3b64a97f6091..25d63731b0e0 100644 > --- a/kernel/audit.h > +++ b/kernel/audit.h > @@ -196,6 +196,12 @@ struct audit_context { > struct { > char *name; > } module; > + struct { > + struct audit_ntp_data data; > + } ntp; > + struct { > + struct timespec64 injoffset; > + } tk; With the ntp and tk structs being separate parts of a union, are we going to have a problem when ADJ_SETOFFSET is set in a call to do_adjtimex()? > }; > int fds[2]; > struct audit_proctitle proctitle; > diff --git a/kernel/auditsc.c b/kernel/auditsc.c > index 6efb0bb909d0..8983790ad86a 100644 > --- a/kernel/auditsc.c > +++ b/kernel/auditsc.c > @@ -1210,11 +1210,18 @@ static void audit_log_fcaps(struct audit_buffer *ab, struct audit_names *name) > from_kuid(&init_user_ns, name->fcap.rootid)); > } > > +void __audit_ntp_log_(const struct audit_ntp_data *ad); > + > static void show_special(struct audit_context *context, int *call_panic) > { > struct audit_buffer *ab; > int i; > > + if (context->type == AUDIT_TIME_ADJNTPVAL) { > + __audit_ntp_log_(&context->ntp.data); > + return; > + } Can we find a way to move this down into the main switch statement in show_special() like you did with AUDIT_TIME_INJOFFSET? This looks *really* hacky to me. Why should AUDIT_TIME_ADJNTPVAL be different from the other "special" bits? > ab = audit_log_start(context, GFP_KERNEL, context->type); > if (!ab) > return; > @@ -1324,6 +1331,11 @@ static void show_special(struct audit_context *context, int *call_panic) > audit_log_format(ab, "(null)"); > > break; > + case AUDIT_TIME_INJOFFSET: > + audit_log_format(ab, "sec=%lli nsec=%li", > + (long long)context->tk.injoffset.tv_sec, > + context->tk.injoffset.tv_nsec); > + break; > } > audit_log_end(ab); > } > @@ -2571,9 +2583,18 @@ void __audit_fanotify(unsigned int response) > > void __audit_tk_injoffset(struct timespec64 offset) > { > - audit_log(audit_context(), GFP_KERNEL, AUDIT_TIME_INJOFFSET, > - "sec=%lli nsec=%li", > - (long long)offset.tv_sec, offset.tv_nsec); > + struct audit_context *context = audit_context(); > + > + context->type = AUDIT_TIME_INJOFFSET; > + memcpy(&context->tk.injoffset, &offset, sizeof(offset)); > +} > + > +void __audit_ntp_log(const struct audit_ntp_data *ad) > +{ > + struct audit_context *context = audit_context(); > + > + context->type = AUDIT_TIME_ADJNTPVAL; > + memcpy(&context->ntp.data, ad, sizeof(*ad)); > } > > static void audit_log_ntp_val(const struct audit_ntp_data *ad, > @@ -2588,7 +2609,7 @@ static void audit_log_ntp_val(const struct audit_ntp_data *ad, > "op=%s old=%lli new=%lli", op, val->oldval, val->newval); > } > > -void __audit_ntp_log(const struct audit_ntp_data *ad) > +void __audit_ntp_log_(const struct audit_ntp_data *ad) > { > audit_log_ntp_val(ad, "offset", AUDIT_NTP_OFFSET); > audit_log_ntp_val(ad, "freq", AUDIT_NTP_FREQ); Ooof, *please* don't end a function, or any symbol for that matter, with an underscore. -- paul moore www.paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH v1] audit: log AUDIT_TIME_* records only from rules 2021-12-29 22:51 ` Paul Moore @ 2022-01-12 21:06 ` Richard Guy Briggs 2022-01-12 21:46 ` Casey Schaufler 2022-01-20 23:07 ` Paul Moore 0 siblings, 2 replies; 14+ messages in thread From: Richard Guy Briggs @ 2022-01-12 21:06 UTC (permalink / raw) To: Paul Moore; +Cc: Eric Paris, Linux-Audit Mailing List On 2021-12-29 17:51, Paul Moore wrote: > On Thu, Nov 4, 2021 at 5:00 PM Richard Guy Briggs <rgb@redhat.com> wrote: > > AUDIT_TIME_* events are generated when there are syscall rules present that are > > not related to time keeping. This will produce noisy log entries that could > > flood the logs and hide events we really care about. > > > > Rather than immediately produce the AUDIT_TIME_* records, store the data and > > log it at syscall exit time respecting the filter rules. > > > > Please see https://bugzilla.redhat.com/show_bug.cgi?id=1991919 > > > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > > --- > > Note: This is a quick and dirty proof-of-concept. If this approach of > > storing the values in the audit_context for later filtering is > > acceptable I'll clean up the patch (re-name functions) and re-submit. > > > > kernel/audit.h | 6 ++++++ > > kernel/auditsc.c | 29 +++++++++++++++++++++++++---- > > 2 files changed, 31 insertions(+), 4 deletions(-) > > Reviewing this now with a more critical eye since it is longer just a > quick-n-dirty proof of concept ... > > > diff --git a/kernel/audit.h b/kernel/audit.h > > index 3b64a97f6091..25d63731b0e0 100644 > > --- a/kernel/audit.h > > +++ b/kernel/audit.h > > @@ -196,6 +196,12 @@ struct audit_context { > > struct { > > char *name; > > } module; > > + struct { > > + struct audit_ntp_data data; > > + } ntp; > > + struct { > > + struct timespec64 injoffset; > > + } tk; > > With the ntp and tk structs being separate parts of a union, are we > going to have a problem when ADJ_SETOFFSET is set in a call to > do_adjtimex()? Yes, so since both record types can exist in one event, either both pieces of data will need to be in one struct within the union, or one piece of data will need to go outside of the union (preferably the smaller one to avoid bloating struct audit_context more than necessary). The tk data is smaller and is easier to check if it is set, so that might be better to store outside the union. Since show_special is keyed off record type, it makes more sense to store one of those datum outside the union and process it outside show_special to avoid record type tricks in show_special. > > }; > > int fds[2]; > > struct audit_proctitle proctitle; > > diff --git a/kernel/auditsc.c b/kernel/auditsc.c > > index 6efb0bb909d0..8983790ad86a 100644 > > --- a/kernel/auditsc.c > > +++ b/kernel/auditsc.c > > @@ -1210,11 +1210,18 @@ static void audit_log_fcaps(struct audit_buffer *ab, struct audit_names *name) > > from_kuid(&init_user_ns, name->fcap.rootid)); > > } > > > > +void __audit_ntp_log_(const struct audit_ntp_data *ad); > > + > > static void show_special(struct audit_context *context, int *call_panic) > > { > > struct audit_buffer *ab; > > int i; > > > > + if (context->type == AUDIT_TIME_ADJNTPVAL) { > > + __audit_ntp_log_(&context->ntp.data); > > + return; > > + } > > Can we find a way to move this down into the main switch statement in > show_special() like you did with AUDIT_TIME_INJOFFSET? This looks > *really* hacky to me. Why should AUDIT_TIME_ADJNTPVAL be different > from the other "special" bits? Agreed, it *looked* hacky to me too. As previously noted, this was the most expedient way to do this due to the unknown number of records being generated, minimum of zero (max 6). show_special allocates an audit_buffer before processing the specific record type, assuming it exists. In the case of audit_ntp_data, it may all be equal and not need to be logged. Ideally, the old vs new data should be compared at the time of the call from do_adjtimex() to find out if we even need to store that data when all old and new values are equal. So, a bit more pre-processing during the call to avoid storing the information if it isn't needed will avoid it at syscall exit. Are you ok with that? > > ab = audit_log_start(context, GFP_KERNEL, context->type); > > if (!ab) > > return; > > @@ -1324,6 +1331,11 @@ static void show_special(struct audit_context *context, int *call_panic) > > audit_log_format(ab, "(null)"); > > > > break; > > + case AUDIT_TIME_INJOFFSET: > > + audit_log_format(ab, "sec=%lli nsec=%li", > > + (long long)context->tk.injoffset.tv_sec, > > + context->tk.injoffset.tv_nsec); > > + break; > > } > > audit_log_end(ab); > > } > > @@ -2571,9 +2583,18 @@ void __audit_fanotify(unsigned int response) > > > > void __audit_tk_injoffset(struct timespec64 offset) > > { > > - audit_log(audit_context(), GFP_KERNEL, AUDIT_TIME_INJOFFSET, > > - "sec=%lli nsec=%li", > > - (long long)offset.tv_sec, offset.tv_nsec); > > + struct audit_context *context = audit_context(); > > + > > + context->type = AUDIT_TIME_INJOFFSET; > > + memcpy(&context->tk.injoffset, &offset, sizeof(offset)); > > +} > > + > > +void __audit_ntp_log(const struct audit_ntp_data *ad) > > +{ > > + struct audit_context *context = audit_context(); > > + > > + context->type = AUDIT_TIME_ADJNTPVAL; > > + memcpy(&context->ntp.data, ad, sizeof(*ad)); > > } > > > > static void audit_log_ntp_val(const struct audit_ntp_data *ad, > > @@ -2588,7 +2609,7 @@ static void audit_log_ntp_val(const struct audit_ntp_data *ad, > > "op=%s old=%lli new=%lli", op, val->oldval, val->newval); > > } > > > > -void __audit_ntp_log(const struct audit_ntp_data *ad) > > +void __audit_ntp_log_(const struct audit_ntp_data *ad) > > { > > audit_log_ntp_val(ad, "offset", AUDIT_NTP_OFFSET); > > audit_log_ntp_val(ad, "freq", AUDIT_NTP_FREQ); > > Ooof, *please* don't end a function, or any symbol for that matter, > with an underscore. Ok, renamed to be consistent with others called from show_special(). > paul moore - RGB -- Richard Guy Briggs <rgb@redhat.com> Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada IRC: rgb, SunRaycer Voice: +1.647.777.2635, Internal: (81) 32635 -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH v1] audit: log AUDIT_TIME_* records only from rules 2022-01-12 21:06 ` Richard Guy Briggs @ 2022-01-12 21:46 ` Casey Schaufler 2022-01-20 23:07 ` Paul Moore 1 sibling, 0 replies; 14+ messages in thread From: Casey Schaufler @ 2022-01-12 21:46 UTC (permalink / raw) To: Richard Guy Briggs, Paul Moore; +Cc: Eric Paris, Linux-Audit Mailing List On 1/12/2022 1:06 PM, Richard Guy Briggs wrote: > On 2021-12-29 17:51, Paul Moore wrote: >> On Thu, Nov 4, 2021 at 5:00 PM Richard Guy Briggs <rgb@redhat.com> wrote: >>> AUDIT_TIME_* events are generated when there are syscall rules present that are >>> not related to time keeping. This will produce noisy log entries that could >>> flood the logs and hide events we really care about. >>> >>> Rather than immediately produce the AUDIT_TIME_* records, store the data and >>> log it at syscall exit time respecting the filter rules. >>> >>> Please see https://bugzilla.redhat.com/show_bug.cgi?id=1991919 >>> >>> Signed-off-by: Richard Guy Briggs <rgb@redhat.com> >>> --- >>> Note: This is a quick and dirty proof-of-concept. If this approach of >>> storing the values in the audit_context for later filtering is >>> acceptable I'll clean up the patch (re-name functions) and re-submit. >>> >>> kernel/audit.h | 6 ++++++ >>> kernel/auditsc.c | 29 +++++++++++++++++++++++++---- >>> 2 files changed, 31 insertions(+), 4 deletions(-) >> Reviewing this now with a more critical eye since it is longer just a >> quick-n-dirty proof of concept ... >> >>> diff --git a/kernel/audit.h b/kernel/audit.h >>> index 3b64a97f6091..25d63731b0e0 100644 >>> --- a/kernel/audit.h >>> +++ b/kernel/audit.h >>> @@ -196,6 +196,12 @@ struct audit_context { >>> struct { >>> char *name; >>> } module; >>> + struct { >>> + struct audit_ntp_data data; >>> + } ntp; >>> + struct { >>> + struct timespec64 injoffset; >>> + } tk; >> With the ntp and tk structs being separate parts of a union, are we >> going to have a problem when ADJ_SETOFFSET is set in a call to >> do_adjtimex()? > Yes, so since both record types can exist in one event, either both > pieces of data will need to be in one struct within the union, or > one piece of data will need to go outside of the union (preferably the > smaller one to avoid bloating struct audit_context more than necessary). > The tk data is smaller and is easier to check if it is set, so that > might be better to store outside the union. > > Since show_special is keyed off record type, it makes more sense to > store one of those datum outside the union and process it outside > show_special to avoid record type tricks in show_special. In the LSM stacking patch set I've implemented a list of auxiliary data elements for supplemental records. That would address these issues and provide the general mechanism we're going to need regardless. > >>> }; >>> int fds[2]; >>> struct audit_proctitle proctitle; >>> diff --git a/kernel/auditsc.c b/kernel/auditsc.c >>> index 6efb0bb909d0..8983790ad86a 100644 >>> --- a/kernel/auditsc.c >>> +++ b/kernel/auditsc.c >>> @@ -1210,11 +1210,18 @@ static void audit_log_fcaps(struct audit_buffer *ab, struct audit_names *name) >>> from_kuid(&init_user_ns, name->fcap.rootid)); >>> } >>> >>> +void __audit_ntp_log_(const struct audit_ntp_data *ad); >>> + >>> static void show_special(struct audit_context *context, int *call_panic) >>> { >>> struct audit_buffer *ab; >>> int i; >>> >>> + if (context->type == AUDIT_TIME_ADJNTPVAL) { >>> + __audit_ntp_log_(&context->ntp.data); >>> + return; >>> + } >> Can we find a way to move this down into the main switch statement in >> show_special() like you did with AUDIT_TIME_INJOFFSET? This looks >> *really* hacky to me. Why should AUDIT_TIME_ADJNTPVAL be different >> from the other "special" bits? > Agreed, it *looked* hacky to me too. As previously noted, this was the > most expedient way to do this due to the unknown number of records being > generated, minimum of zero (max 6). show_special allocates an > audit_buffer before processing the specific record type, assuming it > exists. In the case of audit_ntp_data, it may all be equal and not need > to be logged. > > Ideally, the old vs new data should be compared at the time of the call > from do_adjtimex() to find out if we even need to store that data > when all old and new values are equal. > > So, a bit more pre-processing during the call to avoid storing the > information if it isn't needed will avoid it at syscall exit. Are you > ok with that? > >>> ab = audit_log_start(context, GFP_KERNEL, context->type); >>> if (!ab) >>> return; >>> @@ -1324,6 +1331,11 @@ static void show_special(struct audit_context *context, int *call_panic) >>> audit_log_format(ab, "(null)"); >>> >>> break; >>> + case AUDIT_TIME_INJOFFSET: >>> + audit_log_format(ab, "sec=%lli nsec=%li", >>> + (long long)context->tk.injoffset.tv_sec, >>> + context->tk.injoffset.tv_nsec); >>> + break; >>> } >>> audit_log_end(ab); >>> } >>> @@ -2571,9 +2583,18 @@ void __audit_fanotify(unsigned int response) >>> >>> void __audit_tk_injoffset(struct timespec64 offset) >>> { >>> - audit_log(audit_context(), GFP_KERNEL, AUDIT_TIME_INJOFFSET, >>> - "sec=%lli nsec=%li", >>> - (long long)offset.tv_sec, offset.tv_nsec); >>> + struct audit_context *context = audit_context(); >>> + >>> + context->type = AUDIT_TIME_INJOFFSET; >>> + memcpy(&context->tk.injoffset, &offset, sizeof(offset)); >>> +} >>> + >>> +void __audit_ntp_log(const struct audit_ntp_data *ad) >>> +{ >>> + struct audit_context *context = audit_context(); >>> + >>> + context->type = AUDIT_TIME_ADJNTPVAL; >>> + memcpy(&context->ntp.data, ad, sizeof(*ad)); >>> } >>> >>> static void audit_log_ntp_val(const struct audit_ntp_data *ad, >>> @@ -2588,7 +2609,7 @@ static void audit_log_ntp_val(const struct audit_ntp_data *ad, >>> "op=%s old=%lli new=%lli", op, val->oldval, val->newval); >>> } >>> >>> -void __audit_ntp_log(const struct audit_ntp_data *ad) >>> +void __audit_ntp_log_(const struct audit_ntp_data *ad) >>> { >>> audit_log_ntp_val(ad, "offset", AUDIT_NTP_OFFSET); >>> audit_log_ntp_val(ad, "freq", AUDIT_NTP_FREQ); >> Ooof, *please* don't end a function, or any symbol for that matter, >> with an underscore. > Ok, renamed to be consistent with others called from show_special(). > >> paul moore > - RGB > > -- > Richard Guy Briggs <rgb@redhat.com> > Sr. S/W Engineer, Kernel Security, Base Operating Systems > Remote, Ottawa, Red Hat Canada > IRC: rgb, SunRaycer > Voice: +1.647.777.2635, Internal: (81) 32635 > > -- > Linux-audit mailing list > Linux-audit@redhat.com > https://listman.redhat.com/mailman/listinfo/linux-audit > -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH v1] audit: log AUDIT_TIME_* records only from rules 2022-01-12 21:06 ` Richard Guy Briggs 2022-01-12 21:46 ` Casey Schaufler @ 2022-01-20 23:07 ` Paul Moore 1 sibling, 0 replies; 14+ messages in thread From: Paul Moore @ 2022-01-20 23:07 UTC (permalink / raw) To: Richard Guy Briggs; +Cc: Eric Paris, Linux-Audit Mailing List On Wed, Jan 12, 2022 at 4:06 PM Richard Guy Briggs <rgb@redhat.com> wrote: > On 2021-12-29 17:51, Paul Moore wrote: > > On Thu, Nov 4, 2021 at 5:00 PM Richard Guy Briggs <rgb@redhat.com> wrote: > > > AUDIT_TIME_* events are generated when there are syscall rules present that are > > > not related to time keeping. This will produce noisy log entries that could > > > flood the logs and hide events we really care about. > > > > > > Rather than immediately produce the AUDIT_TIME_* records, store the data and > > > log it at syscall exit time respecting the filter rules. > > > > > > Please see https://bugzilla.redhat.com/show_bug.cgi?id=1991919 > > > > > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > > > --- > > > Note: This is a quick and dirty proof-of-concept. If this approach of > > > storing the values in the audit_context for later filtering is > > > acceptable I'll clean up the patch (re-name functions) and re-submit. > > > > > > kernel/audit.h | 6 ++++++ > > > kernel/auditsc.c | 29 +++++++++++++++++++++++++---- > > > 2 files changed, 31 insertions(+), 4 deletions(-) > > > > Reviewing this now with a more critical eye since it is longer just a > > quick-n-dirty proof of concept ... > > > > > diff --git a/kernel/audit.h b/kernel/audit.h > > > index 3b64a97f6091..25d63731b0e0 100644 > > > --- a/kernel/audit.h > > > +++ b/kernel/audit.h > > > @@ -196,6 +196,12 @@ struct audit_context { > > > struct { > > > char *name; > > > } module; > > > + struct { > > > + struct audit_ntp_data data; > > > + } ntp; > > > + struct { > > > + struct timespec64 injoffset; > > > + } tk; > > > > With the ntp and tk structs being separate parts of a union, are we > > going to have a problem when ADJ_SETOFFSET is set in a call to > > do_adjtimex()? > > Yes, so since both record types can exist in one event, either both > pieces of data will need to be in one struct within the union, or > one piece of data will need to go outside of the union (preferably the > smaller one to avoid bloating struct audit_context more than necessary). > The tk data is smaller and is easier to check if it is set, so that > might be better to store outside the union. > > Since show_special is keyed off record type, it makes more sense to > store one of those datum outside the union and process it outside > show_special to avoid record type tricks in show_special. It seems so much easier just to put both "data" and "injoffset" inside a single struct within the union, and considering the size of mq_getsetattr in that same union I doubt the size will increase significantly (or at all, if my quick estimation stands up). I believe our best approach is to stay consistent with how we handle other things similar to this, I don't currently see any reason why the NTP adjustments would require special handling. As an aside, how did you test this and not run into this issue? > > > diff --git a/kernel/auditsc.c b/kernel/auditsc.c > > > index 6efb0bb909d0..8983790ad86a 100644 > > > --- a/kernel/auditsc.c > > > +++ b/kernel/auditsc.c > > > @@ -1210,11 +1210,18 @@ static void audit_log_fcaps(struct audit_buffer *ab, struct audit_names *name) > > > from_kuid(&init_user_ns, name->fcap.rootid)); > > > } > > > > > > +void __audit_ntp_log_(const struct audit_ntp_data *ad); > > > + > > > static void show_special(struct audit_context *context, int *call_panic) > > > { > > > struct audit_buffer *ab; > > > int i; > > > > > > + if (context->type == AUDIT_TIME_ADJNTPVAL) { > > > + __audit_ntp_log_(&context->ntp.data); > > > + return; > > > + } > > > > Can we find a way to move this down into the main switch statement in > > show_special() like you did with AUDIT_TIME_INJOFFSET? This looks > > *really* hacky to me. Why should AUDIT_TIME_ADJNTPVAL be different > > from the other "special" bits? > > Agreed, it *looked* hacky to me too. As previously noted, this was the > most expedient way to do this due to the unknown number of records being > generated, minimum of zero (max 6). show_special allocates an > audit_buffer before processing the specific record type, assuming it > exists. In the case of audit_ntp_data, it may all be equal and not need > to be logged. > > Ideally, the old vs new data should be compared at the time of the call > from do_adjtimex() to find out if we even need to store that data > when all old and new values are equal. > > So, a bit more pre-processing during the call to avoid storing the > information if it isn't needed will avoid it at syscall exit. Are you > ok with that? The currently proposed approach is ugly enough that I think it is worth looking at alternatives, let's see what you can come up with in the next version and make a decision then. > > > @@ -2588,7 +2609,7 @@ static void audit_log_ntp_val(const struct audit_ntp_data *ad, > > > "op=%s old=%lli new=%lli", op, val->oldval, val->newval); > > > } > > > > > > -void __audit_ntp_log(const struct audit_ntp_data *ad) > > > +void __audit_ntp_log_(const struct audit_ntp_data *ad) > > > { > > > audit_log_ntp_val(ad, "offset", AUDIT_NTP_OFFSET); > > > audit_log_ntp_val(ad, "freq", AUDIT_NTP_FREQ); > > > > Ooof, *please* don't end a function, or any symbol for that matter, > > with an underscore. > > Ok, renamed to be consistent with others called from show_special(). Thanks. -- paul moore paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2022-01-20 23:08 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-11-04 20:59 [RFC PATCH v1] audit: log AUDIT_TIME_* records only from rules Richard Guy Briggs 2021-11-04 21:29 ` Paul Moore 2021-11-04 21:53 ` Richard Guy Briggs 2021-11-19 16:15 ` Paul Moore 2021-11-19 18:01 ` Richard Guy Briggs 2021-11-24 15:44 ` Paul Moore 2021-12-21 20:50 ` Richard Guy Briggs 2021-12-23 17:38 ` Paul Moore 2021-12-24 14:21 ` Richard Guy Briggs 2021-12-27 15:07 ` Paul Moore 2021-12-29 22:51 ` Paul Moore 2022-01-12 21:06 ` Richard Guy Briggs 2022-01-12 21:46 ` Casey Schaufler 2022-01-20 23:07 ` Paul Moore
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).