| Message ID | 20230726113901.546569-1-ming.lei@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ublk: fail to start device if queue setup is interrupted | expand |
On Wed, Jul 26, 2023 at 07:39:01PM +0800, Ming Lei wrote: >In ublk_ctrl_start_dev(), if wait_for_completion_interruptible() is >interrupted by signal, queues aren't setup successfully yet, so we >have to fail UBLK_CMD_START_DEV, otherwise kernel oops can be triggered. > >Reported by German when working for supporting ublk on qemu-storage-deamon >which requires single thread ublk daemon. > >Fixes: 71f28f3136af ("ublk_drv: add io_uring based userspace block driver") >Reported-by: German Maglione <gmaglione@redhat.com> >Signed-off-by: Ming Lei <ming.lei@redhat.com> >--- > drivers/block/ublk_drv.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > >diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c >index 1c823750c95a..7938221f4f7e 100644 >--- a/drivers/block/ublk_drv.c >+++ b/drivers/block/ublk_drv.c >@@ -1847,7 +1847,8 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub, struct io_uring_cmd *cmd) > if (ublksrv_pid <= 0) > return -EINVAL; > >- wait_for_completion_interruptible(&ub->completion); >+ if (wait_for_completion_interruptible(&ub->completion) != 0) >+ return -EINTR; Should we do somenthig similar also in ublk_ctrl_end_recovery()? Maybe also in ublk_ctrl_del_dev() we can return -EINTR. Thanks, Stefano
On Wed, Jul 26, 2023 at 03:30:34PM +0200, Stefano Garzarella wrote: > On Wed, Jul 26, 2023 at 07:39:01PM +0800, Ming Lei wrote: > > In ublk_ctrl_start_dev(), if wait_for_completion_interruptible() is > > interrupted by signal, queues aren't setup successfully yet, so we > > have to fail UBLK_CMD_START_DEV, otherwise kernel oops can be triggered. > > > > Reported by German when working for supporting ublk on qemu-storage-deamon > > which requires single thread ublk daemon. > > > > Fixes: 71f28f3136af ("ublk_drv: add io_uring based userspace block driver") > > Reported-by: German Maglione <gmaglione@redhat.com> > > Signed-off-by: Ming Lei <ming.lei@redhat.com> > > --- > > drivers/block/ublk_drv.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c > > index 1c823750c95a..7938221f4f7e 100644 > > --- a/drivers/block/ublk_drv.c > > +++ b/drivers/block/ublk_drv.c > > @@ -1847,7 +1847,8 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub, struct io_uring_cmd *cmd) > > if (ublksrv_pid <= 0) > > return -EINVAL; > > > > - wait_for_completion_interruptible(&ub->completion); > > + if (wait_for_completion_interruptible(&ub->completion) != 0) > > + return -EINTR; > > Should we do somenthig similar also in ublk_ctrl_end_recovery()? Good catch, ublk_ctrl_end_recovery() do need similar handling, otherwise similar kernel oops may be triggered too. > > Maybe also in ublk_ctrl_del_dev() we can return -EINTR. It doesn't matter for ublk_ctrl_del_dev() given it just waits for existed users, but still good to return -EINTR to user. Thanks, Ming
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c index 1c823750c95a..7938221f4f7e 100644 --- a/drivers/block/ublk_drv.c +++ b/drivers/block/ublk_drv.c @@ -1847,7 +1847,8 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub, struct io_uring_cmd *cmd) if (ublksrv_pid <= 0) return -EINVAL; - wait_for_completion_interruptible(&ub->completion); + if (wait_for_completion_interruptible(&ub->completion) != 0) + return -EINTR; schedule_delayed_work(&ub->monitor_work, UBLK_DAEMON_MONITOR_PERIOD);
In ublk_ctrl_start_dev(), if wait_for_completion_interruptible() is interrupted by signal, queues aren't setup successfully yet, so we have to fail UBLK_CMD_START_DEV, otherwise kernel oops can be triggered. Reported by German when working for supporting ublk on qemu-storage-deamon which requires single thread ublk daemon. Fixes: 71f28f3136af ("ublk_drv: add io_uring based userspace block driver") Reported-by: German Maglione <gmaglione@redhat.com> Signed-off-by: Ming Lei <ming.lei@redhat.com> --- drivers/block/ublk_drv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)