fix? current_io_context() vs set_task_ioprio() race
Subsystems (2)
PATCH 1/1 fix? current_io_context() vs set_task_ioprio() race
I know nothing about io scheduler, but I suspect set_task_ioprio() is not safe.
current_io_context() initializes "struct io_context", then sets ->io_context. set_task_ioprio() running on another cpu may see the changes out of order, so ->set_ioprio(ioc) may use io_context which was not initialized properly.
block/ll_rw_blk.c renamed from block/ll_rw_blk.c~3_race +2 -0
@@ -3628,6 +3628,8 @@ struct io_context *current_io_context(gf ret->nr_batch_requests =0 ;/* because this is 0 */ ret->aic =NULL ; ret->cic_root.rb_node =NULL ; +/* make sure set_task_ioprio() sees the settings above */ +smp_wmb (); tsk->io_context = ret; }
fs/ioprio.c renamed from fs/ioprio.c~3_race +3 -0
@@ -44,6 +44,9 @@ static int set_task_ioprio(struct task_s task->ioprio = ioprio; ioc = task->io_context; +/* see wmb() in current_io_context() */ +smp_read_barrier_depends (); +if (ioc && ioc->set_ioprio) ioc->set_ioprio (ioc, ioprio);
Quoted 22 lines · Oleg Nesterov
I know nothing about io scheduler, but I suspect
set_task_ioprio()is not safe.
current_io_context()initializes "structio_context", then sets ->io_context.set_task_ioprio()running on another cpu may see the changes out of order, so ->set_ioprio(ioc) may useio_contextwhich was not initialized properly.Signed-off-by Oleg Nesterov <[email protected]>block/ll_rw_blk.c renamed from block/ll_rw_blk.c~3_race +2
-0@@ -3628,6 +3628,8 @@ struct io_context *current_io_context(gf ret->nr_batch_requests =0 ;/* because this is 0 */ ret->aic =NULL ; ret->cic_root.rb_node =NULL ; +/* make sure set_task_ioprio() sees the settings above */ +smp_wmb (); tsk->io_context = ret; }
fs/ioprio.c static int set_task_ioprio(struct task_s · line 51
Quoted 8 lines
fs/ioprio.c renamed from fs/ioprio.c~3_race +3
-0@@ -44,6 +44,9 @@ static int set_task_ioprio(struct task_s task->ioprio = ioprio; ioc = task->io_context; +/* see wmb() in current_io_context() */ +smp_read_barrier_depends (); +if (ioc && ioc->set_ioprio) ioc->set_ioprio (ioc, ioprio);
Agree, applied.
Signature · 1 line
Jens Axboe