audit.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Paul Moore <paul@paul-moore.com>
To: Eiichi Tsukata <eiichi.tsukata@nutanix.com>
Cc: "eparis@redhat.com" <eparis@redhat.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"audit@vger.kernel.org" <audit@vger.kernel.org>
Subject: Re: [PATCH v2 5/5] audit: do not use exclusive wait in audit_receive()
Date: Tue, 23 May 2023 17:07:28 -0400	[thread overview]
Message-ID: <CAHC9VhQ73CcDtO7DAaRaN3aJiSBn_XQLAO83iGuGchky5hOvDg@mail.gmail.com> (raw)
In-Reply-To: <F900B719-7760-4E22-82A2-933ED775AA19@nutanix.com>

On Mon, May 22, 2023 at 11:58 PM Eiichi Tsukata
<eiichi.tsukata@nutanix.com> wrote:
> > On May 22, 2023, at 13:44, Eiichi Tsukata <eiichi.tsukata@nutanix.com> wrote:
> >> 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:
> >>>
> >>> kauditd thread issues wake_up() before it goes to sleep. The wake_up()
> >>> call wakes up only one process as waiter side uses exclusive wait.
> >>> This can be problematic when there are multiple processes (one is in
> >>> audit_receive() and others are in audit_log_start()) waiting on
> >>> audit_backlog_wait queue.
> >>>
> >>> For example, if there are two processes waiting:
> >>>
> >>> Process (A): in audit_receive()
> >>> Process (B): in audit_log_start()
> >>>
> >>> And (A) is at the head of the wait queue. Then kauditd's wake_up() only
> >>> wakes up (A) leaving (B) as it is even if @audit_queue is drained. As a
> >>> result, (B) can be blocked for up to backlog_wait_time.
> >>>
> >>> To prevent the issue, use non-exclusive wait in audit_receive() so that
> >>> kauditd can wake up all waiters in audit_receive().
> >>>
> >>> Fixes: 8f110f530635 ("audit: ensure userspace is penalized the same as the kernel when under pressure")
> >>> Signed-off-by: Eiichi Tsukata <eiichi.tsukata@nutanix.com>
> >>> ---
> >>> kernel/audit.c | 17 +++++++++++------
> >>> 1 file changed, 11 insertions(+), 6 deletions(-)
> >>
> >> This was also discussed in the last patchset.
> >
> > This bug is much easily reproducible on real environments and can cause problematic
> > user space failure like SSH connection timeout.
> > Let’s not keep the bug unfixed.
> > (Of course we’ve already carefully tuned audit related params and user space auditd config so that our product won’t hit backlog full.)

Good.  Resolving your issues through audit runtime configuration is
the proper solution to this.

> > BTW, the default backlog_wait_time is 60 * HZ which seems pretty large.
> > I’d appreciate if you could tell me the reason behind that value.

I do not recall the original logic behind that value.  It is likely
that the original value predated my maintenance of the audit
subsystem.

-- 
paul-moore.com

      reply	other threads:[~2023-05-23 21:07 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-11  5:21 [PATCH v2 0/5] audit: refactors and fixes for potential deadlocks Eiichi Tsukata
2023-05-11  5:21 ` [PATCH v2 1/5] audit: refactor queue full checks Eiichi Tsukata
2023-05-19 20:54   ` Paul Moore
2023-05-22  4:19     ` Eiichi Tsukata
2023-05-11  5:21 ` [PATCH v2 2/5] audit: account backlog waiting time in audit_receive() Eiichi Tsukata
2023-05-19 20:54   ` Paul Moore
2023-05-22  4:22     ` Eiichi Tsukata
2023-05-23 20:55       ` Paul Moore
2023-05-11  5:21 ` [PATCH v2 3/5] audit: convert DECLARE_WAITQUEUE to DEFINE_WAIT Eiichi Tsukata
2023-05-19 20:54   ` Paul Moore
2023-05-11  5:21 ` [PATCH v2 4/5] audit: check if audit_queue is full after prepare_to_wait_exclusive() Eiichi Tsukata
2023-05-19 20:54   ` Paul Moore
2023-05-22  4:28     ` Eiichi Tsukata
2023-05-23 21:02       ` Paul Moore
2023-05-11  5:21 ` [PATCH v2 5/5] audit: do not use exclusive wait in audit_receive() Eiichi Tsukata
2023-05-19 20:54   ` Paul Moore
2023-05-22  4:44     ` Eiichi Tsukata
2023-05-23  3:58       ` Eiichi Tsukata
2023-05-23 21:07         ` Paul Moore [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAHC9VhQ73CcDtO7DAaRaN3aJiSBn_XQLAO83iGuGchky5hOvDg@mail.gmail.com \
    --to=paul@paul-moore.com \
    --cc=audit@vger.kernel.org \
    --cc=eiichi.tsukata@nutanix.com \
    --cc=eparis@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).