2023-05-11 05:24:03

by Eiichi Tsukata

Subject: [PATCH v2 1/5] audit: refactor queue full checks

Currently audit queue full checks are done in multiple places.
Consolidate them into one audit_queue_full().

Signed-off-by: Eiichi Tsukata <[email protected]>
---
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);
+}
+
/**
* 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);

--
2.40.0

2023-05-19 21:02:59

by Paul Moore

Subject: Re: [PATCH v2 1/5] audit: refactor queue full checks

On May 11, 2023 Eiichi Tsukata <[email protected]> wrote:
>
> Currently audit queue full checks are done in multiple places.
> Consolidate them into one audit_queue_full().
>
> Signed-off-by: Eiichi Tsukata <[email protected]>
> ---
> 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

2023-05-22 04:46:00

by Eiichi Tsukata

Subject: Re: [PATCH v2 1/5] audit: refactor queue full checks

> On May 20, 2023, at 5:54, Paul Moore <[email protected]> wrote:
>
> On May 11, 2023 Eiichi Tsukata <[email protected]> wrote:
>>
>> Currently audit queue full checks are done in multiple places.
>> Consolidate them into one audit_queue_full().
>>
>> Signed-off-by: Eiichi Tsukata <[email protected]>
>> ---
>> 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