kernel/fork.c | 2 +- kernel/pid.c | 8 ++++---- kernel/pid_namespace.c | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-)
Suppress syzbot reports by annotating these accesses using data_race().
Reported-by: syzbot+adcaa842b762a1762e7d@syzkaller.appspotmail.com
Reported-by: syzbot+fab52e3459fa2f95df57@syzkaller.appspotmail.com
Reported-by: syzbot+0718f65353d72efaac1e@syzkaller.appspotmail.com
Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
kernel/fork.c | 2 +-
kernel/pid.c | 8 ++++----
kernel/pid_namespace.c | 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/kernel/fork.c b/kernel/fork.c
index c4b26cd8998b..5aa050418fda 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2584,7 +2584,7 @@ __latent_entropy struct task_struct *copy_process(
rseq_fork(p, clone_flags);
/* Don't start children in a dying pid namespace */
- if (unlikely(!(ns_of_pid(pid)->pid_allocated & PIDNS_ADDING))) {
+ if (unlikely(!(data_race(ns_of_pid(pid)->pid_allocated & PIDNS_ADDING)))) {
retval = -ENOMEM;
goto bad_fork_core_free;
}
diff --git a/kernel/pid.c b/kernel/pid.c
index 4ac2ce46817f..df59228dd27e 100644
--- a/kernel/pid.c
+++ b/kernel/pid.c
@@ -122,7 +122,7 @@ void free_pid(struct pid *pid)
for (i = 0; i <= pid->level; i++) {
struct upid *upid = pid->numbers + i;
struct pid_namespace *ns = upid->ns;
- switch (--ns->pid_allocated) {
+ switch (data_race(--ns->pid_allocated)) {
case 2:
case 1:
/* When all that is left in the pid namespace
@@ -134,7 +134,7 @@ void free_pid(struct pid *pid)
case PIDNS_ADDING:
/* Handle a fork failure of the first process */
WARN_ON(ns->child_reaper);
- ns->pid_allocated = 0;
+ data_race(ns->pid_allocated = 0);
break;
}
@@ -271,13 +271,13 @@ struct pid *alloc_pid(struct pid_namespace *ns, pid_t *set_tid,
upid = pid->numbers + ns->level;
idr_preload(GFP_KERNEL);
spin_lock(&pidmap_lock);
- if (!(ns->pid_allocated & PIDNS_ADDING))
+ if (!(data_race(ns->pid_allocated & PIDNS_ADDING)))
goto out_unlock;
pidfs_add_pid(pid);
for ( ; upid >= pid->numbers; --upid) {
/* Make the PID visible to find_pid_ns. */
idr_replace(&upid->ns->idr, pid, upid->nr);
- upid->ns->pid_allocated++;
+ data_race(upid->ns->pid_allocated++);
}
spin_unlock(&pidmap_lock);
idr_preload_end();
diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
index 7098ed44e717..6e2a32641904 100644
--- a/kernel/pid_namespace.c
+++ b/kernel/pid_namespace.c
@@ -268,7 +268,7 @@ void zap_pid_ns_processes(struct pid_namespace *pid_ns)
*/
for (;;) {
set_current_state(TASK_INTERRUPTIBLE);
- if (pid_ns->pid_allocated == init_pids)
+ if (data_race(pid_ns->pid_allocated == init_pids))
break;
schedule();
}
--
2.47.1On 04/25, Jiayuan Chen wrote:
>
> @@ -2584,7 +2584,7 @@ __latent_entropy struct task_struct *copy_process(
> rseq_fork(p, clone_flags);
>
> /* Don't start children in a dying pid namespace */
> - if (unlikely(!(ns_of_pid(pid)->pid_allocated & PIDNS_ADDING))) {
> + if (unlikely(!(data_race(ns_of_pid(pid)->pid_allocated & PIDNS_ADDING)))) {
Well. data_race() just hides the potential problem. READ_ONCE() makes more
sense imo, even if I think there are no real problems with the current code.
Either way,
> @@ -271,13 +271,13 @@ struct pid *alloc_pid(struct pid_namespace *ns, pid_t *set_tid,
> upid = pid->numbers + ns->level;
> idr_preload(GFP_KERNEL);
> spin_lock(&pidmap_lock);
> - if (!(ns->pid_allocated & PIDNS_ADDING))
> + if (!(data_race(ns->pid_allocated & PIDNS_ADDING)))
again, you do not need data_race() or READ_ONCE() if you read the
data protected by pidmap_lock. But you still need WRITE_ONCE() when
->pid_allocated is modified.
Oleg.
On Fri, Apr 25, 2025 at 01:58:14PM +0800, Jiayuan Chen <jiayuan.chen@linux.dev> wrote: > Suppress syzbot reports by annotating these accesses using data_race(). Thanks for trying this approach. scripts/checkpatch.pl has quite some remarks about the current form :-) I mean, the data_race annotation should document why the race is harmless. I only glanced over this so I can't tell myself whether it's a bug or OK. > Reported-by: syzbot+adcaa842b762a1762e7d@syzkaller.appspotmail.com > Reported-by: syzbot+fab52e3459fa2f95df57@syzkaller.appspotmail.com > Reported-by: syzbot+0718f65353d72efaac1e@syzkaller.appspotmail.com How can I get to see full syzbot reports? (Stacktrace would be helpful, I cannot resolve those as message-ids on (LK)ML.) Thanks, Michal
April 25, 2025 at 17:25, "Michal Koutný" <mkoutny@suse.com> wrote: > > On Fri, Apr 25, 2025 at 01:58:14PM +0800, Jiayuan Chen <jiayuan.chen@linux.dev> wrote: > > > > > Suppress syzbot reports by annotating these accesses using data_race(). > > > > Thanks for trying this approach. > > scripts/checkpatch.pl has quite some remarks about the current form :-) > > I mean, the data_race annotation should document why the race is > > harmless. I only glanced over this so I can't tell myself whether it's a > > bug or OK. > > > > > Reported-by: syzbot+adcaa842b762a1762e7d@syzkaller.appspotmail.com > > > > Reported-by: syzbot+fab52e3459fa2f95df57@syzkaller.appspotmail.com > > > > Reported-by: syzbot+0718f65353d72efaac1e@syzkaller.appspotmail.com > > > > How can I get to see full syzbot reports? (Stacktrace would be helpful, > > I cannot resolve those as message-ids on (LK)ML.) > > Thanks, > > Michal > Hi Michal, We can find such reports at https://syzkaller.appspot.com/upstream/s/kernel
© 2016 - 2025 Red Hat, Inc.