From: Paul Moore <paul@paul-moore.com>
To: abhishek.shah@columbia.edu
Cc: linux-audit@redhat.com, linux-kernel@vger.kernel.org,
eparis@redhat.com, Gabriel Ryan <gabe@cs.columbia.edu>
Subject: Re: data-race in audit_log_start / audit_receive
Date: Thu, 18 Aug 2022 21:59:02 -0400 [thread overview]
Message-ID: <CAHC9VhTXNPWBDRoPcz-Jw=f+NNAEhxbh-ySc56CUd-ZbuboW5w@mail.gmail.com> (raw)
In-Reply-To: <CAEHB248ft0LFUQDHNtB9NN_D3F=12Jndh13Ue=LokajXNhMk5Q@mail.gmail.com>
On Thu, Aug 18, 2022 at 6:23 PM Abhishek Shah
<abhishek.shah@columbia.edu> wrote:
> Hi all,
>
> We found a data race involving the audit_cmd_mutex.owner variable. We think this bug is concerning because audit_ctl_owner_current is used at a location that controls the scheduling of tasks shown here. Please let us know what you think.
>
> Thanks!
>
> -----------------Report----------------------
>
> write to 0xffffffff881d0710 of 8 bytes by task 6541 on cpu 0:
> audit_ctl_lock kernel/audit.c:237 [inline]
...
> read to 0xffffffff881d0710 of 8 bytes by task 6542 on cpu 1:
> audit_ctl_owner_current kernel/audit.c:258 [inline]
Yes, technically there is a race condition if/when an auditd instance
is registering itself the exact same time as another task is
attempting to log an audit record via audit_log_start(). The risk
being that a *very* limited number of audit records could be
mis-handled with respect to their queue priority and that is it; no
records would be lost or misplaced. Correcting this would likely
involve a more complex locking scheme[1] or a rather severe
performance penalty due to an additional lock in the audit_log_start()
code path. There may be some value in modifying
audit_ctl_owner_current() to use READ_ONCE(), but it isn't clear to me
that this would significantly improve things or have no impact on
performance.
Have you noticed any serious problems on your system due to this? If
you have a reproducer which shows actual harm on the system could you
please share that?
[1] The obvious choice would be to move to a RCU based scheme, but
even that doesn't totally solve the problem as there would still be a
window where some tasks would have an "old" value. It might actually
end up extending the race window on large multi-core systems due to
the time needed for all of the critical sections to complete.
--
paul-moore.com
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit
next prev parent reply other threads:[~2022-08-19 1:59 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-18 22:23 data-race in audit_log_start / audit_receive Abhishek Shah
2022-08-19 1:59 ` Paul Moore [this message]
2022-08-19 12:06 ` Paul Moore
2022-08-22 20:09 ` Gabriel Ryan
2022-08-22 23:42 ` Paul Moore
2022-08-23 13:09 ` Gabriel Ryan
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='CAHC9VhTXNPWBDRoPcz-Jw=f+NNAEhxbh-ySc56CUd-ZbuboW5w@mail.gmail.com' \
--to=paul@paul-moore.com \
--cc=abhishek.shah@columbia.edu \
--cc=eparis@redhat.com \
--cc=gabe@cs.columbia.edu \
--cc=linux-audit@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).