Re: [PATCH V4 18/24] block: fail to show/store elevator sysfs attribute if elevator is dying

[Ming Lei <ming.lei@redhat.com>]

On Wed, Apr 30, 2025 at 08:31:01AM +0200, Hannes Reinecke wrote:
> On 4/30/25 06:35, Ming Lei wrote:
> > Prepare for moving elv_register[unregister]_queue out of elevator_lock
> > & queue freezing, so we may have to call elv_unregister_queue() after
> > elevator ->exit() is called, then there is small window for user to
> > call into ->show()/store(), and user-after-free can be caused.
> > 
> > Fail to show/store elevator sysfs attribute if elevator is dying by
> > adding one new flag of ELEVATOR_FLAG_DYNG, which is protected by
> > elevator ->sysfs_lock.
> > 
> > Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
> > Signed-off-by: Ming Lei <ming.lei@redhat.com>
> > ---
> >   block/blk-mq-sched.c |  1 +
> >   block/elevator.c     | 10 ++++++----
> >   block/elevator.h     |  1 +
> >   3 files changed, 8 insertions(+), 4 deletions(-)
> > 
> > diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
> > index 336a15ffecfa..55a0fd105147 100644
> > --- a/block/blk-mq-sched.c
> > +++ b/block/blk-mq-sched.c
> > @@ -551,5 +551,6 @@ void blk_mq_exit_sched(struct request_queue *q, struct elevator_queue *e)
> >   	if (e->type->ops.exit_sched)
> >   		e->type->ops.exit_sched(e);
> >   	blk_mq_sched_tags_teardown(q, flags);
> > +	set_bit(ELEVATOR_FLAG_DYING, &q->elevator->flags);
> >   	q->elevator = NULL;
> >   }
> 
> set_bit() is unordered; don't you need to take ->sysfs_lock here?

Yes.

blk_mq_exit_sched() is called from elevator_exit() with eq->sysfs_lock held.


thanks,
Ming