__get_task_ioprio() reads task->io_context and ioc->ioprio without task_lock() when called on current, e.g. from init_sync_kiocb() on every write. set_task_ioprio() can update both for another task at any time, including the first publication of a new io_context. KCSAN reports it: BUG: KCSAN: data-race in __kernel_write_iter / set_task_ioprio value changed: 0x0000000000000000 -> 0xffff888109813730 A stale or torn value is harmless, a task simply gets the old or new priority, but the compiler may tear or refetch the accesses and the io_context fields must be visible before the pointer is. The io_context itself needs no extra protection: only the owning task drops it, in exit_io_context(), so it cannot be freed under a lockless reader running as current. Publish the pointer with smp_store_release(), read it with READ_ONCE() (address dependency orders the ioprio load), and mark the ioprio accesses. Reported-by: syzbot+feeb9b5352b9ff89618a@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=feeb9b5352b9ff89618a Signed-off-by: Nguyen Ngoc Thang --- v2: - Document why no RCU is needed: only the owning task drops its io_context (Damien). Comment in __get_task_ioprio() and a paragraph in the commit message. No code change. - v1: https://lore.kernel.org/r/20261018071636.79198-1-ngocthang2710.1999@gmail.com Tested with a KCSAN kernel in QEMU: 26 data-race reports per run before, 0 in two runs after (v1 code, v2 differs only in comments). block/blk-ioc.c | 5 +++-- include/linux/ioprio.h | 9 +++++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/block/blk-ioc.c b/block/blk-ioc.c index d15918d7fabb..12ddc672f41d 100644 --- a/block/blk-ioc.c +++ b/block/blk-ioc.c @@ -277,9 +277,10 @@ int set_task_ioprio(struct task_struct *task, int ioprio) if (task->io_context) kmem_cache_free(iocontext_cachep, ioc); else - task->io_context = ioc; + /* Pairs with READ_ONCE() in __get_task_ioprio() */ + smp_store_release(&task->io_context, ioc); } - task->io_context->ioprio = ioprio; + WRITE_ONCE(task->io_context->ioprio, ioprio); out: task_unlock(task); return 0; diff --git a/include/linux/ioprio.h b/include/linux/ioprio.h index 5210e8371238..d148548691c7 100644 --- a/include/linux/ioprio.h +++ b/include/linux/ioprio.h @@ -56,7 +56,12 @@ static inline int task_nice_ioclass(struct task_struct *task) */ static inline int __get_task_ioprio(struct task_struct *p) { - struct io_context *ioc = p->io_context; + /* + * Only the task itself drops its io_context (exit_io_context()), so + * for p == current a non-NULL ioc stays valid. Another task can + * still install it or change ->ioprio, hence the annotations. + */ + struct io_context *ioc = READ_ONCE(p->io_context); int prio; if (!ioc) @@ -66,7 +71,7 @@ static inline int __get_task_ioprio(struct task_struct *p) if (p != current) lockdep_assert_held(&p->alloc_lock); - prio = ioc->ioprio; + prio = READ_ONCE(ioc->ioprio); if (IOPRIO_PRIO_CLASS(prio) == IOPRIO_CLASS_NONE) prio = IOPRIO_PRIO_VALUE(task_nice_ioclass(p), task_nice_ioprio(p)); -- 2.43.0