All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] audit: annotate the lockless read of the audit control lock owner
@ 2026-10-02  1:17 Babanpreet Singh
  2026-10-02  9:14 ` sashiko-bot
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ 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 related	[flat|nested] 7+ 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  9:14 ` sashiko-bot
  2026-10-02 14:44 ` Paul Moore
  2026-10-08  3:44 ` [PATCH v2] " Babanpreet Singh
  2 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-10-02  9:14 UTC (permalink / raw)
  Cc: audit

> 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>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261002011712.7-1-bbnpreetsingh@gmail.com?part=1


^ permalink raw reply	[flat|nested] 7+ 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  9:14 ` sashiko-bot
@ 2026-10-02 14:44 ` Paul Moore
  2026-10-08  3:44 ` [PATCH v2] " Babanpreet Singh
  2 siblings, 0 replies; 7+ 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] 7+ messages in thread

* [PATCH v2] audit: annotate the lockless read of the audit control lock owner
@ 2026-10-08  3:44 ` Babanpreet Singh
  2026-10-08  3:48   ` sashiko-bot
                     ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Babanpreet Singh @ 2026-10-08  3:44 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>
---
v2: drop the comment in audit_ctl_owner_current(). The KCSAN build of
audit.o disassembles the same as v1.
v1: https://lore.kernel.org/all/20261002011712.7-1-bbnpreetsingh@gmail.com/
Compile tested only (gcc W=1 and the KCSAN instrumentation diff); I
could not reproduce the race in QEMU.

 kernel/audit.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index 631d6d4a23cfd..d0c2036bde759 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,7 @@ void audit_ctl_unlock(void)
  */
 static bool audit_ctl_owner_current(void)
 {
-	return (current == audit_cmd_mutex.owner);
+	return (current == READ_ONCE(audit_cmd_mutex.owner));
 }
 
 /**

base-commit: a8bdcf944504980635c2069b4a4cfcf677b09bbb
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] audit: annotate the lockless read of the audit control lock owner
  2026-10-08  3:44 ` [PATCH v2] " Babanpreet Singh
@ 2026-10-08  3:48   ` sashiko-bot
  2026-10-08 15:03   ` Bradley Morgan
  2026-10-30  3:33   ` Babanpreet Singh
  2 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-10-08  3:48 UTC (permalink / raw)
  Cc: audit

> 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>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008034403.7-1-bbnpreetsingh@gmail.com?part=1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] audit: annotate the lockless read of the audit control lock owner
  2026-10-08  3:44 ` [PATCH v2] " Babanpreet Singh
  2026-10-08  3:48   ` sashiko-bot
@ 2026-10-08 15:03   ` Bradley Morgan
  2026-10-30  3:33   ` Babanpreet Singh
  2 siblings, 0 replies; 7+ messages in thread
From: Bradley Morgan @ 2026-10-08 15:03 UTC (permalink / raw)
  To: bbnpreetsingh
  Cc: audit, eparis, linux-kernel, paul, syzbot+39b8fea0641107a1ccee

On 8 October 2026 04:44:03 BST, 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

Reviewed-by: Bradley Morgan <brads@mainlining.org>

Tested-by: Bradley Morgan <brads@mainlining.org> # Power10


>Signed-off-by: Babanpreet Singh <bbnpreetsingh@gmail.com>
>---
>v2: drop the comment in audit_ctl_owner_current(). The KCSAN build of
>audit.o disassembles the same as v1.
>v1: https://lore.kernel.org/all/20261002011712.7-1-bbnpreetsingh@gmail.com/
>Compile tested only (gcc W=1 and the KCSAN instrumentation diff); I
>could not reproduce the race in QEMU.
>
> kernel/audit.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/kernel/audit.c b/kernel/audit.c
>index 631d6d4a23cfd..d0c2036bde759 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,7 @@ void audit_ctl_unlock(void)
>  */
> static bool audit_ctl_owner_current(void)
> {
>-	return (current == audit_cmd_mutex.owner);
>+	return (current == READ_ONCE(audit_cmd_mutex.owner));
> }
> 
> /**

Clean!

>
>base-commit: a8bdcf944504980635c2069b4a4cfcf677b09bbb
>

--- Thanks!
"I'm not a very positive person" - Linus torvalds

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] audit: annotate the lockless read of the audit control lock owner
  2026-10-08  3:44 ` [PATCH v2] " Babanpreet Singh
  2026-10-08  3:48   ` sashiko-bot
  2026-10-08 15:03   ` Bradley Morgan
@ 2026-10-30  3:33   ` Babanpreet Singh
  2 siblings, 0 replies; 7+ messages in thread
From: Babanpreet Singh @ 2026-10-30  3:33 UTC (permalink / raw)
  To: Paul Moore; +Cc: Eric Paris, Bradley Morgan, audit, linux-kernel, bbnpreetsingh

Hi Paul,

Gentle ping on this v2. It has been three weeks, Bradley gave it a
Reviewed-by and Tested-by, and I don't see it in audit/dev or audit/next
yet. If you want something else changed, tell me and I will respin.

Thanks,
Babanpreet

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-10-30  3:33 UTC | newest]

Thread overview: 7+ 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  9:14 ` sashiko-bot
2026-10-02 14:44 ` Paul Moore
2026-10-08  3:44 ` [PATCH v2] " Babanpreet Singh
2026-10-08  3:48   ` sashiko-bot
2026-10-08 15:03   ` Bradley Morgan
2026-10-30  3:33   ` Babanpreet Singh

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.