* [PATCH] audit: annotate the lockless read of the audit control lock owner
@ 2026-10-02 1:17 Babanpreet Singh
2026-10-02 14:44 ` Paul Moore
0 siblings, 1 reply; 2+ messages in thread
From: Babanpreet Singh @ 2026-10-02 1:17 UTC (permalink / raw)
To: Paul Moore, Eric Paris
Cc: audit, linux-kernel, Babanpreet Singh, syzbot+39b8fea0641107a1ccee
audit_ctl_owner_current() reads the lock owner without taking the lock
and KCSAN complains about it. Only the current task can store itself as
owner, so the check can't give a wrong answer. Mark the accesses with
READ_ONCE() and WRITE_ONCE().
Reported-by: syzbot+39b8fea0641107a1ccee@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=39b8fea0641107a1ccee
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Babanpreet Singh <bbnpreetsingh@gmail.com>
---
Paul, you looked at this race in 2022 and asked whether READ_ONCE()
was worth it and what it would cost:
https://lore.kernel.org/all/CAHC9VhTXNPWBDRoPcz-Jw=f+NNAEhxbh-ySc56CUd-ZbuboW5w@mail.gmail.com/
Compile tested only (gcc and the KCSAN instrumentation diff); I could
not reproduce the race in QEMU.
kernel/audit.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/kernel/audit.c b/kernel/audit.c
index 631d6d4a23cfd..f8c43ac6d5b82 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -247,7 +247,7 @@ int auditd_test_task(struct task_struct *task)
void audit_ctl_lock(void)
{
mutex_lock(&audit_cmd_mutex.lock);
- audit_cmd_mutex.owner = current;
+ WRITE_ONCE(audit_cmd_mutex.owner, current);
}
/**
@@ -255,7 +255,7 @@ void audit_ctl_lock(void)
*/
void audit_ctl_unlock(void)
{
- audit_cmd_mutex.owner = NULL;
+ WRITE_ONCE(audit_cmd_mutex.owner, NULL);
mutex_unlock(&audit_cmd_mutex.lock);
}
@@ -268,7 +268,12 @@ void audit_ctl_unlock(void)
*/
static bool audit_ctl_owner_current(void)
{
- return (current == audit_cmd_mutex.owner);
+ /*
+ * Lockless read: the owner can only equal current if current set
+ * it, so another task changing the owner concurrently can never
+ * make this return the wrong answer.
+ */
+ return (current == READ_ONCE(audit_cmd_mutex.owner));
}
/**
base-commit: a8bdcf944504980635c2069b4a4cfcf677b09bbb
--
2.43.0
^ permalink raw reply [flat|nested] 2+ messages in thread* Re: [PATCH] audit: annotate the lockless read of the audit control lock owner
2026-10-02 1:17 [PATCH] audit: annotate the lockless read of the audit control lock owner Babanpreet Singh
@ 2026-10-02 14:44 ` Paul Moore
0 siblings, 0 replies; 2+ messages in thread
From: Paul Moore @ 2026-10-02 14:44 UTC (permalink / raw)
To: Babanpreet Singh
Cc: Eric Paris, audit, linux-kernel, syzbot+39b8fea0641107a1ccee
On Thu, Oct 1, 2026 at 9:17 PM Babanpreet Singh <bbnpreetsingh@gmail.com> wrote:
>
> audit_ctl_owner_current() reads the lock owner without taking the lock
> and KCSAN complains about it. Only the current task can store itself as
> owner, so the check can't give a wrong answer. Mark the accesses with
> READ_ONCE() and WRITE_ONCE().
>
> Reported-by: syzbot+39b8fea0641107a1ccee@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=39b8fea0641107a1ccee
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Babanpreet Singh <bbnpreetsingh@gmail.com>
> ---
> Paul, you looked at this race in 2022 and asked whether READ_ONCE()
> was worth it and what it would cost:
> https://lore.kernel.org/all/CAHC9VhTXNPWBDRoPcz-Jw=f+NNAEhxbh-ySc56CUd-ZbuboW5w@mail.gmail.com/
> Compile tested only (gcc and the KCSAN instrumentation diff); I could
> not reproduce the race in QEMU.
>
> kernel/audit.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
Thanks for the patch.
I stand by my previous comments, but given the current state of LLM
assisted reporting and submissions it's probably worth making the
change. I would like to see one small change, see below ...
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 631d6d4a23cfd..f8c43ac6d5b82 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -268,7 +268,12 @@ void audit_ctl_unlock(void)
> */
> static bool audit_ctl_owner_current(void)
> {
> - return (current == audit_cmd_mutex.owner);
> + /*
> + * Lockless read: the owner can only equal current if current set
> + * it, so another task changing the owner concurrently can never
> + * make this return the wrong answer.
> + */
This code is fairly easy to understand so I don't believe this comment
is necessary or adds any value, please drop this commment and
resubmit.
> + return (current == READ_ONCE(audit_cmd_mutex.owner));
> }
--
paul-moore.com
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-10-02 14:44 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-02 1:17 [PATCH] audit: annotate the lockless read of the audit control lock owner Babanpreet Singh
2026-10-02 14:44 ` Paul Moore
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®