[Qemu-devel] [PATCH v2] nbd: release exp->blk after all clients are closed

[Qemu-devel] [PATCH v2] nbd: release exp->blk after all clients are closed

[Wen Congyang]

If the socket fd is shutdown, there may be some data which is received before
shutdown. We will read the data and do read/write in nbd_trip(). But the exp's
blk is NULL, and it will cause qemu crashed.

Reported-by: Li Zhijian <lizhijian@cn.fujitsu.com>
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 nbd.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/nbd.c b/nbd.c
index 06b501b..07240bd 100644
--- a/nbd.c
+++ b/nbd.c
@@ -1131,12 +1131,6 @@ void nbd_export_close(NBDExport *exp)
     }
     nbd_export_set_name(exp, NULL);
     nbd_export_put(exp);
-    if (exp->blk) {
-        blk_remove_aio_context_notifier(exp->blk, blk_aio_attached,
-                                        blk_aio_detach, exp);
-        blk_unref(exp->blk);
-        exp->blk = NULL;
-    }
 }
 
 void nbd_export_get(NBDExport *exp)
@@ -1159,6 +1153,13 @@ void nbd_export_put(NBDExport *exp)
             exp->close(exp);
         }
 
+        if (exp->blk) {
+            blk_remove_aio_context_notifier(exp->blk, blk_aio_attached,
+                                            blk_aio_detach, exp);
+            blk_unref(exp->blk);
+            exp->blk = NULL;
+        }
+
         g_free(exp);
     }
 }
@@ -1305,6 +1306,14 @@ static void nbd_trip(void *opaque)
         goto invalid_request;
     }
 
+    if (client->closing) {
+        /*
+         * The client may be closed when we are blocked in
+         * nbd_co_receive_request()
+         */
+        goto done;
+    }
+
     switch (command) {
     case NBD_CMD_READ:
         TRACE("Request type is READ");
-- 
2.4.3

Re: [Qemu-devel] [PATCH v2] nbd: release exp->blk after all clients are closed

[Paolo Bonzini]

On 16/09/2015 10:35, Wen Congyang wrote:
> If the socket fd is shutdown, there may be some data which is received before
> shutdown. We will read the data and do read/write in nbd_trip(). But the exp's
> blk is NULL, and it will cause qemu crashed.
> 
> Reported-by: Li Zhijian <lizhijian@cn.fujitsu.com>
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>

Thanks, this looks good.  I have added

Cc: qemu-stable@nongnu.org

and will send a pull request soon.

Paolo

> ---
>  nbd.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/nbd.c b/nbd.c
> index 06b501b..07240bd 100644
> --- a/nbd.c
> +++ b/nbd.c
> @@ -1131,12 +1131,6 @@ void nbd_export_close(NBDExport *exp)
>      }
>      nbd_export_set_name(exp, NULL);
>      nbd_export_put(exp);
> -    if (exp->blk) {
> -        blk_remove_aio_context_notifier(exp->blk, blk_aio_attached,
> -                                        blk_aio_detach, exp);
> -        blk_unref(exp->blk);
> -        exp->blk = NULL;
> -    }
>  }
>  
>  void nbd_export_get(NBDExport *exp)
> @@ -1159,6 +1153,13 @@ void nbd_export_put(NBDExport *exp)
>              exp->close(exp);
>          }
>  
> +        if (exp->blk) {
> +            blk_remove_aio_context_notifier(exp->blk, blk_aio_attached,
> +                                            blk_aio_detach, exp);
> +            blk_unref(exp->blk);
> +            exp->blk = NULL;
> +        }
> +
>          g_free(exp);
>      }
>  }
> @@ -1305,6 +1306,14 @@ static void nbd_trip(void *opaque)
>          goto invalid_request;
>      }
>  
> +    if (client->closing) {
> +        /*
> +         * The client may be closed when we are blocked in
> +         * nbd_co_receive_request()
> +         */
> +        goto done;
> +    }
> +
>      switch (command) {
>      case NBD_CMD_READ:
>          TRACE("Request type is READ");
>