[PATCH 0/1] Fix for one more race on scsi device removal

[PATCH 0/1] Fix for one more race on scsi device removal

[Maxim Levitsky]

This is a patch from Paulo that he suggested to fix bz #1854811.
(https://bugzilla.redhat.com/show_bug.cgi?id=1854811)

I think that the race that is described in this bug can still happen (in theory)
so it is better to plug it with a refcount as it was suggested.

Best regards,
	Maxim Levitsky

Maxim Levitsky (1):
  scsi: fix device removal race vs IO restart callback on resume

 hw/scsi/scsi-bus.c | 4 ++++
 1 file changed, 4 insertions(+)

-- 
2.26.2

[PATCH 1/1] scsi: fix device removal race vs IO restart callback on resume

[Maxim Levitsky]

There is (mostly theoretical) race between removal of a scsi device and
scsi_dma_restart_bh.

It used to be easier to hit this race prior to my / Paulo's patch series
that added rcu to scsi bus device handling code, but IMHO this race
should still be possible to hit, at least in theory.

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1854811

Fix it anyway with a patch that was proposed by Paulo in the above bugzilla.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 hw/scsi/scsi-bus.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index b901e701f0..edb5c3492a 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -170,6 +170,8 @@ static void scsi_dma_restart_bh(void *opaque)
         scsi_req_unref(req);
     }
     aio_context_release(blk_get_aio_context(s->conf.blk));
+    /* Drop the reference that was acquired in scsi_dma_restart_cb */
+    object_unref(OBJECT(s));
 }
 
 void scsi_req_retry(SCSIRequest *req)
@@ -188,6 +190,8 @@ static void scsi_dma_restart_cb(void *opaque, int running, RunState state)
     }
     if (!s->bh) {
         AioContext *ctx = blk_get_aio_context(s->conf.blk);
+        /* The reference is dropped in scsi_dma_restart_bh.*/
+        object_ref(OBJECT(s));
         s->bh = aio_bh_new(ctx, scsi_dma_restart_bh, s);
         qemu_bh_schedule(s->bh);
     }
-- 
2.26.2

Re: [PATCH 1/1] scsi: fix device removal race vs IO restart callback on resume

[Paolo Bonzini]

On 10/12/20 13:59, Maxim Levitsky wrote:
> There is (mostly theoretical) race between removal of a scsi device and
> scsi_dma_restart_bh.
> 
> It used to be easier to hit this race prior to my / Paulo's patch series
> that added rcu to scsi bus device handling code, but IMHO this race
> should still be possible to hit, at least in theory.
> 
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1854811
> 
> Fix it anyway with a patch that was proposed by Paulo in the above bugzilla.
> 
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
>   hw/scsi/scsi-bus.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
> index b901e701f0..edb5c3492a 100644
> --- a/hw/scsi/scsi-bus.c
> +++ b/hw/scsi/scsi-bus.c
> @@ -170,6 +170,8 @@ static void scsi_dma_restart_bh(void *opaque)
>           scsi_req_unref(req);
>       }
>       aio_context_release(blk_get_aio_context(s->conf.blk));
> +    /* Drop the reference that was acquired in scsi_dma_restart_cb */
> +    object_unref(OBJECT(s));
>   }
>   
>   void scsi_req_retry(SCSIRequest *req)
> @@ -188,6 +190,8 @@ static void scsi_dma_restart_cb(void *opaque, int running, RunState state)
>       }
>       if (!s->bh) {
>           AioContext *ctx = blk_get_aio_context(s->conf.blk);
> +        /* The reference is dropped in scsi_dma_restart_bh.*/
> +        object_ref(OBJECT(s));
>           s->bh = aio_bh_new(ctx, scsi_dma_restart_bh, s);
>           qemu_bh_schedule(s->bh);
>       }
> 

Queued, thanks.

Paolo