| Message ID | 20230511052116.19452-2-eiichi.tsukata@nutanix.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | audit: refactors and fixes for potential deadlocks | expand |
On May 11, 2023 Eiichi Tsukata <eiichi.tsukata@nutanix.com> wrote: > > Currently audit queue full checks are done in multiple places. > Consolidate them into one audit_queue_full(). > > Signed-off-by: Eiichi Tsukata <eiichi.tsukata@nutanix.com> > --- > kernel/audit.c | 21 +++++++++++---------- > 1 file changed, 11 insertions(+), 10 deletions(-) > > diff --git a/kernel/audit.c b/kernel/audit.c > index 9bc0b0301198..c15694e1a76b 100644 > --- a/kernel/audit.c > +++ b/kernel/audit.c > @@ -341,6 +341,12 @@ static inline int audit_rate_check(void) > return retval; > } > > +static inline int audit_queue_full(const struct sk_buff_head *queue) > +{ > + return audit_backlog_limit && > + (skb_queue_len(queue) > audit_backlog_limit); > +} Regardless of the other patches in this series, this seems like a good candidate to merge, but could you make it return a 'bool' instead of an 'int'? -- paul-moore.com
> On May 20, 2023, at 5:54, Paul Moore <paul@paul-moore.com> wrote: > > On May 11, 2023 Eiichi Tsukata <eiichi.tsukata@nutanix.com> wrote: >> >> Currently audit queue full checks are done in multiple places. >> Consolidate them into one audit_queue_full(). >> >> Signed-off-by: Eiichi Tsukata <eiichi.tsukata@nutanix.com> >> --- >> kernel/audit.c | 21 +++++++++++---------- >> 1 file changed, 11 insertions(+), 10 deletions(-) >> >> diff --git a/kernel/audit.c b/kernel/audit.c >> index 9bc0b0301198..c15694e1a76b 100644 >> --- a/kernel/audit.c >> +++ b/kernel/audit.c >> @@ -341,6 +341,12 @@ static inline int audit_rate_check(void) >> return retval; >> } >> >> +static inline int audit_queue_full(const struct sk_buff_head *queue) >> +{ >> + return audit_backlog_limit && >> + (skb_queue_len(queue) > audit_backlog_limit); >> +} > > Regardless of the other patches in this series, this seems like a > good candidate to merge, but could you make it return a 'bool' > instead of an 'int'? > Sure, will fix it in v3. Eiichi
diff --git a/kernel/audit.c b/kernel/audit.c index 9bc0b0301198..c15694e1a76b 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -341,6 +341,12 @@ static inline int audit_rate_check(void) return retval; } +static inline int audit_queue_full(const struct sk_buff_head *queue) +{ + return audit_backlog_limit && + (skb_queue_len(queue) > audit_backlog_limit); +} + /** * audit_log_lost - conditionally log lost audit message event * @message: the message stating reason for lost audit message @@ -579,8 +585,7 @@ static void kauditd_hold_skb(struct sk_buff *skb, int error) * record on the retry queue unless it's full, in which case drop it */ if (error == -EAGAIN) { - if (!audit_backlog_limit || - skb_queue_len(&audit_retry_queue) < audit_backlog_limit) { + if (!audit_queue_full(&audit_retry_queue)) { skb_queue_tail(&audit_retry_queue, skb); return; } @@ -589,8 +594,7 @@ static void kauditd_hold_skb(struct sk_buff *skb, int error) } /* if we have room in the hold queue, queue the message */ - if (!audit_backlog_limit || - skb_queue_len(&audit_hold_queue) < audit_backlog_limit) { + if (!audit_queue_full(&audit_hold_queue)) { skb_queue_tail(&audit_hold_queue, skb); return; } @@ -613,8 +617,7 @@ static void kauditd_hold_skb(struct sk_buff *skb, int error) */ static void kauditd_retry_skb(struct sk_buff *skb, __always_unused int error) { - if (!audit_backlog_limit || - skb_queue_len(&audit_retry_queue) < audit_backlog_limit) { + if (!audit_queue_full(&audit_retry_queue)) { skb_queue_tail(&audit_retry_queue, skb); return; } @@ -1564,8 +1567,7 @@ static void audit_receive(struct sk_buff *skb) audit_ctl_unlock(); /* can't block with the ctrl lock, so penalize the sender now */ - if (audit_backlog_limit && - (skb_queue_len(&audit_queue) > audit_backlog_limit)) { + if (audit_queue_full(&audit_queue)) { DECLARE_WAITQUEUE(wait, current); /* wake kauditd to try and flush the queue */ @@ -1866,8 +1868,7 @@ struct audit_buffer *audit_log_start(struct audit_context *ctx, gfp_t gfp_mask, if (!(auditd_test_task(current) || audit_ctl_owner_current())) { long stime = audit_backlog_wait_time; - while (audit_backlog_limit && - (skb_queue_len(&audit_queue) > audit_backlog_limit)) { + while (audit_queue_full(&audit_queue)) { /* wake kauditd to try and flush the queue */ wake_up_interruptible(&kauditd_wait);
Currently audit queue full checks are done in multiple places. Consolidate them into one audit_queue_full(). Signed-off-by: Eiichi Tsukata <eiichi.tsukata@nutanix.com> --- kernel/audit.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-)