Messages in this thread | | | From | Paul Moore <> | Date | Tue, 28 Dec 2021 20:20:51 -0500 | Subject | Re: [PATCH -next 1/1] audit: ignore userspace log when backlog limit exceeded and backlog_wait_time is 0 |
| |
On Tue, Dec 28, 2021 at 1:50 AM luhuaxin <luhuaxin1@huawei.com> wrote: > > When the backlog exceed the backlog_limit and backlog_wait_time is set > to 0, the process will only sleep for a very short time (jiffies). The > backlog may still exceed backlog_limit in extreme cases. > > The more reasonable way to fix this problem is: > > 1. If backlog_wait_time is set to zero, ignore the log; > 2. If backlog_wait_time is set to non-zero, let process sleep for > backlog_wait_time. > > The above log limit logic is also the same as that in the existing > audit_log_start function. > > Fixes: 8f110f530635 ("[PATCH] audit: ensure userspace is penalized the > same as the kernel when under pressure")
One quick comment on the "Fixes" tag above: you shouldn't add the "[PATCH]" string to the commit's subject, you should use the commit subject that you would see if you typed `git log --oneline`. It also shouldn't be word-wrapped, it should be all on one line in your patch/email.
Regardless of the above, I don't think this is a patch we want to merge upstream. I can understand the desire to improve performance, but this doesn't seem appropriate to me; adjusting the backlog_wait_time to very low values just so you can drop audit records is not an approach we want to advocate, or support, upstream.
> Signed-off-by: luhuaxin <luhuaxin1@huawei.com> > --- > kernel/audit.c | 14 +++++++++++--- > 1 file changed, 11 insertions(+), 3 deletions(-) > > diff --git a/kernel/audit.c b/kernel/audit.c > index 249e11628..70450f70a 100644 > --- a/kernel/audit.c > +++ b/kernel/audit.c > @@ -1545,7 +1545,8 @@ static void audit_receive(struct sk_buff *skb) > > /* can't block with the ctrl lock, so penalize the sender now */ > if (audit_backlog_limit && > - (skb_queue_len(&audit_queue) > audit_backlog_limit)) { > + (skb_queue_len(&audit_queue) > audit_backlog_limit) && > + audit_backlog_wait_time) { > DECLARE_WAITQUEUE(wait, current); > > /* wake kauditd to try and flush the queue */ > @@ -1842,9 +1843,8 @@ struct audit_buffer *audit_log_start(struct audit_context *ctx, gfp_t gfp_mask, > * while holding the mutex, although we do penalize the sender > * later in audit_receive() when it is safe to block > */ > + long stime = audit_backlog_wait_time; > 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)) { > /* wake kauditd to try and flush the queue */ > @@ -1872,6 +1872,14 @@ struct audit_buffer *audit_log_start(struct audit_context *ctx, gfp_t gfp_mask, > return NULL; > } > } > + } else if (!stime && audit_backlog_limit && > + (skb_queue_len(&audit_queue) > audit_backlog_limit)) { > + if (audit_rate_check() && printk_ratelimit()) > + pr_warn("audit_backlog=%d > audit_backlog_limit=%d\n", > + skb_queue_len(&audit_queue), > + audit_backlog_limit); > + audit_log_lost("backlog limit exceeded"); > + return NULL; > } > > ab = audit_buffer_alloc(ctx, gfp_mask, type); > -- > 2.23.0
-- paul moore www.paul-moore.com
| |