diff mbox series

[1/1] vhost-scsi: fix usage of error_reportf_err()

Message ID 20231214003117.43960-1-dongli.zhang@oracle.com
State New
Headers show
Series [1/1] vhost-scsi: fix usage of error_reportf_err() | expand

Commit Message

Dongli Zhang Dec. 14, 2023, 12:31 a.m. UTC 
It is required to use error_report() instead of error_reportf_err(), if the
prior function does not take local_err as the argument. As a result, the
local_err is always NULL and segment fault may happen.

vhost_scsi_start()
-> vhost_scsi_set_endpoint(s) --> does not allocate local_err
-> error_reportf_err()
   -> error_vprepend()
      -> g_string_append(newmsg, (*errp)->msg) --> (*errp) is NULL

In addition, add ": " at the end of other error_reportf_err() logs.

Fixes: 7962e432b4e4 ("vhost-user-scsi: support reconnect to backend")
Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
---
 hw/scsi/vhost-scsi.c      | 4 ++--
 hw/scsi/vhost-user-scsi.c | 3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

Comments

Li Feng Dec. 14, 2023, 3:25 a.m. UTC | #1 
> On Dec 14, 2023, at 08:31, Dongli Zhang <dongli.zhang@oracle.com> wrote:
> 
> It is required to use error_report() instead of error_reportf_err(), if the
> prior function does not take local_err as the argument. As a result, the
> local_err is always NULL and segment fault may happen.
> 
> vhost_scsi_start()
> -> vhost_scsi_set_endpoint(s) --> does not allocate local_err
> -> error_reportf_err()
>   -> error_vprepend()
>      -> g_string_append(newmsg, (*errp)->msg) --> (*errp) is NULL
> 
> In addition, add ": " at the end of other error_reportf_err() logs.
> 
> Fixes: 7962e432b4e4 ("vhost-user-scsi: support reconnect to backend")
> Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
> ---
> hw/scsi/vhost-scsi.c      | 4 ++--
> hw/scsi/vhost-user-scsi.c | 3 ++-
> 2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
> index 3126df9e1d..9929c0d14b 100644
> --- a/hw/scsi/vhost-scsi.c
> +++ b/hw/scsi/vhost-scsi.c
> @@ -91,13 +91,13 @@ static int vhost_scsi_start(VHostSCSI *s)
> 
>     ret = vhost_scsi_common_start(vsc, &local_err);
>     if (ret < 0) {
> -        error_reportf_err(local_err, "Error starting vhost-scsi");
> +        error_reportf_err(local_err, "Error starting vhost-scsi: ");
>         return ret;
>     }
> 
>     ret = vhost_scsi_set_endpoint(s);
>     if (ret < 0) {
> -        error_reportf_err(local_err, "Error setting vhost-scsi endpoint");
> +        error_report("Error setting vhost-scsi endpoint");
>         vhost_scsi_common_stop(vsc);
>     }
> 
> diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
> index 780f10559d..af18c4f3d3 100644
> --- a/hw/scsi/vhost-user-scsi.c
> +++ b/hw/scsi/vhost-user-scsi.c
> @@ -83,7 +83,8 @@ static void vhost_user_scsi_set_status(VirtIODevice *vdev, uint8_t status)
>     if (should_start) {
>         ret = vhost_user_scsi_start(s, &local_err);
>         if (ret < 0) {
> -            error_reportf_err(local_err, "unable to start vhost-user-scsi: %s",
> +            error_reportf_err(local_err,
> +                              "unable to start vhost-user-scsi: %s: ",
>                               strerror(-ret));
>             qemu_chr_fe_disconnect(&vs->conf.chardev);
>         }
> -- 
> 2.34.1
> 
Looks good.

Reviewed-by: Feng Li <fengli@smartx.com>

Thanks.
Raphael Norwitz Dec. 14, 2023, 3:55 p.m. UTC | #2 
> On Dec 13, 2023, at 7:31 PM, Dongli Zhang <dongli.zhang@oracle.com> wrote:
> 
> It is required to use error_report() instead of error_reportf_err(), if the
> prior function does not take local_err as the argument. As a result, the
> local_err is always NULL and segment fault may happen.
> 
> vhost_scsi_start()
> -> vhost_scsi_set_endpoint(s) --> does not allocate local_err
> -> error_reportf_err()
>   -> error_vprepend()
>      -> g_string_append(newmsg, (*errp)->msg) --> (*errp) is NULL
> 
> In addition, add ": " at the end of other error_reportf_err() logs.
> 
> Fixes: 7962e432b4e4 ("vhost-user-scsi: support reconnect to backend")
> Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>

Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com>

> ---
> hw/scsi/vhost-scsi.c      | 4 ++--
> hw/scsi/vhost-user-scsi.c | 3 ++-
> 2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
> index 3126df9e1d..9929c0d14b 100644
> --- a/hw/scsi/vhost-scsi.c
> +++ b/hw/scsi/vhost-scsi.c
> @@ -91,13 +91,13 @@ static int vhost_scsi_start(VHostSCSI *s)
> 
>     ret = vhost_scsi_common_start(vsc, &local_err);
>     if (ret < 0) {
> -        error_reportf_err(local_err, "Error starting vhost-scsi");
> +        error_reportf_err(local_err, "Error starting vhost-scsi: ");
>         return ret;
>     }
> 
>     ret = vhost_scsi_set_endpoint(s);
>     if (ret < 0) {
> -        error_reportf_err(local_err, "Error setting vhost-scsi endpoint");
> +        error_report("Error setting vhost-scsi endpoint");
>         vhost_scsi_common_stop(vsc);
>     }
> 
> diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
> index 780f10559d..af18c4f3d3 100644
> --- a/hw/scsi/vhost-user-scsi.c
> +++ b/hw/scsi/vhost-user-scsi.c
> @@ -83,7 +83,8 @@ static void vhost_user_scsi_set_status(VirtIODevice *vdev, uint8_t status)
>     if (should_start) {
>         ret = vhost_user_scsi_start(s, &local_err);
>         if (ret < 0) {
> -            error_reportf_err(local_err, "unable to start vhost-user-scsi: %s",
> +            error_reportf_err(local_err,
> +                              "unable to start vhost-user-scsi: %s: ",
>                               strerror(-ret));
>             qemu_chr_fe_disconnect(&vs->conf.chardev);
>         }
> -- 
> 2.34.1
>
diff mbox series

Patch

diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
index 3126df9e1d..9929c0d14b 100644
--- a/hw/scsi/vhost-scsi.c
+++ b/hw/scsi/vhost-scsi.c
@@ -91,13 +91,13 @@  static int vhost_scsi_start(VHostSCSI *s)
 
     ret = vhost_scsi_common_start(vsc, &local_err);
     if (ret < 0) {
-        error_reportf_err(local_err, "Error starting vhost-scsi");
+        error_reportf_err(local_err, "Error starting vhost-scsi: ");
         return ret;
     }
 
     ret = vhost_scsi_set_endpoint(s);
     if (ret < 0) {
-        error_reportf_err(local_err, "Error setting vhost-scsi endpoint");
+        error_report("Error setting vhost-scsi endpoint");
         vhost_scsi_common_stop(vsc);
     }
 
diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
index 780f10559d..af18c4f3d3 100644
--- a/hw/scsi/vhost-user-scsi.c
+++ b/hw/scsi/vhost-user-scsi.c
@@ -83,7 +83,8 @@  static void vhost_user_scsi_set_status(VirtIODevice *vdev, uint8_t status)
     if (should_start) {
         ret = vhost_user_scsi_start(s, &local_err);
         if (ret < 0) {
-            error_reportf_err(local_err, "unable to start vhost-user-scsi: %s",
+            error_reportf_err(local_err,
+                              "unable to start vhost-user-scsi: %s: ",
                               strerror(-ret));
             qemu_chr_fe_disconnect(&vs->conf.chardev);
         }