1. Patchew
  2. QEMU
  3. View series

[PATCH] vhost-user-blk: Fix two resource leaks

AlexChen posted 1 patch 5 years, 3 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/5F997B84.9040900@huawei.com
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Raphael Norwitz <raphael.norwitz@nutanix.com>
contrib/vhost-user-blk/vhost-user-blk.c   | 2 +-
contrib/vhost-user-scsi/vhost-user-scsi.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
[PATCH] vhost-user-blk: Fix two resource leaks
Posted by AlexChen 5 years, 3 months ago 
When socket() fails, it returns -1, 0 is the normal return value and should not return

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: AlexChen <alex.chen@huawei.com>
---
 contrib/vhost-user-blk/vhost-user-blk.c   | 2 +-
 contrib/vhost-user-scsi/vhost-user-scsi.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/vhost-user-blk/vhost-user-blk.c b/contrib/vhost-user-blk/vhost-user-blk.c
index 25eccd02b5..40a2dfc544 100644
--- a/contrib/vhost-user-blk/vhost-user-blk.c
+++ b/contrib/vhost-user-blk/vhost-user-blk.c
@@ -474,7 +474,7 @@ static int unix_sock_new(char *unix_fn)
     assert(unix_fn);

     sock = socket(AF_UNIX, SOCK_STREAM, 0);
-    if (sock <= 0) {
+    if (sock < 0) {
         perror("socket");
         return -1;
     }
diff --git a/contrib/vhost-user-scsi/vhost-user-scsi.c b/contrib/vhost-user-scsi/vhost-user-scsi.c
index 3c912384e9..0f9ba4b2a2 100644
--- a/contrib/vhost-user-scsi/vhost-user-scsi.c
+++ b/contrib/vhost-user-scsi/vhost-user-scsi.c
@@ -320,7 +320,7 @@ static int unix_sock_new(char *unix_fn)
     assert(unix_fn);

     sock = socket(AF_UNIX, SOCK_STREAM, 0);
-    if (sock <= 0) {
+    if (sock < 0) {
         perror("socket");
         return -1;
     }
-- 
2.19.1
Re: [PATCH] vhost-user-blk: Fix two resource leaks
Posted by Raphael Norwitz 5 years, 3 months ago 
The change looks good but I'm not sure I'd call it resource leak in
either case since the failure case kills vhost-user-blk/scsi. In the
commit message maybe rather say "vhost-user-blk/scsi: fix broken error
handling for socket call"?

On Wed, Oct 28, 2020 at 10:10 AM AlexChen <alex.chen@huawei.com> wrote:
>
> When socket() fails, it returns -1, 0 is the normal return value and should not return
>
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: AlexChen <alex.chen@huawei.com>
> ---
>  contrib/vhost-user-blk/vhost-user-blk.c   | 2 +-
>  contrib/vhost-user-scsi/vhost-user-scsi.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/vhost-user-blk/vhost-user-blk.c b/contrib/vhost-user-blk/vhost-user-blk.c
> index 25eccd02b5..40a2dfc544 100644
> --- a/contrib/vhost-user-blk/vhost-user-blk.c
> +++ b/contrib/vhost-user-blk/vhost-user-blk.c
> @@ -474,7 +474,7 @@ static int unix_sock_new(char *unix_fn)
>      assert(unix_fn);
>
>      sock = socket(AF_UNIX, SOCK_STREAM, 0);
> -    if (sock <= 0) {
> +    if (sock < 0) {
>          perror("socket");
>          return -1;
>      }
> diff --git a/contrib/vhost-user-scsi/vhost-user-scsi.c b/contrib/vhost-user-scsi/vhost-user-scsi.c
> index 3c912384e9..0f9ba4b2a2 100644
> --- a/contrib/vhost-user-scsi/vhost-user-scsi.c
> +++ b/contrib/vhost-user-scsi/vhost-user-scsi.c
> @@ -320,7 +320,7 @@ static int unix_sock_new(char *unix_fn)
>      assert(unix_fn);
>
>      sock = socket(AF_UNIX, SOCK_STREAM, 0);
> -    if (sock <= 0) {
> +    if (sock < 0) {
>          perror("socket");
>          return -1;
>      }
> --
> 2.19.1
>
Re: [PATCH] vhost-user-blk: Fix two resource leaks
Posted by AlexChen 5 years, 3 months ago 
On 2020/10/28 23:40, Raphael Norwitz wrote:
> The change looks good but I'm not sure I'd call it resource leak in
> either case since the failure case kills vhost-user-blk/scsi. In the
> commit message maybe rather say "vhost-user-blk/scsi: fix broken error
> handling for socket call"?
>

Thanks for your suggestion. I will modify the commit message in next version.

Thanks,
Alex

> On Wed, Oct 28, 2020 at 10:10 AM AlexChen <alex.chen@huawei.com> wrote:
>>
>> When socket() fails, it returns -1, 0 is the normal return value and should not return
>>
>> Reported-by: Euler Robot <euler.robot@huawei.com>
>> Signed-off-by: AlexChen <alex.chen@huawei.com>
>> ---
>>  contrib/vhost-user-blk/vhost-user-blk.c   | 2 +-
>>  contrib/vhost-user-scsi/vhost-user-scsi.c | 2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/contrib/vhost-user-blk/vhost-user-blk.c b/contrib/vhost-user-blk/vhost-user-blk.c
>> index 25eccd02b5..40a2dfc544 100644
>> --- a/contrib/vhost-user-blk/vhost-user-blk.c
>> +++ b/contrib/vhost-user-blk/vhost-user-blk.c
>> @@ -474,7 +474,7 @@ static int unix_sock_new(char *unix_fn)
>>      assert(unix_fn);
>>
>>      sock = socket(AF_UNIX, SOCK_STREAM, 0);
>> -    if (sock <= 0) {
>> +    if (sock < 0) {
>>          perror("socket");
>>          return -1;
>>      }
>> diff --git a/contrib/vhost-user-scsi/vhost-user-scsi.c b/contrib/vhost-user-scsi/vhost-user-scsi.c
>> index 3c912384e9..0f9ba4b2a2 100644
>> --- a/contrib/vhost-user-scsi/vhost-user-scsi.c
>> +++ b/contrib/vhost-user-scsi/vhost-user-scsi.c
>> @@ -320,7 +320,7 @@ static int unix_sock_new(char *unix_fn)
>>      assert(unix_fn);
>>
>>      sock = socket(AF_UNIX, SOCK_STREAM, 0);
>> -    if (sock <= 0) {
>> +    if (sock < 0) {
>>          perror("socket");
>>          return -1;
>>      }
>> --
>> 2.19.1
>>
> .
>
[PATCH V2] vhost-user-blk/scsi: Fix broken error handling for socket call
Posted by AlexChen 5 years, 3 months ago 
When socket() fails, it returns -1, 0 is the normal return value and should not return error.

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: AlexChen <alex.chen@huawei.com>
---
 contrib/vhost-user-blk/vhost-user-blk.c   | 2 +-
 contrib/vhost-user-scsi/vhost-user-scsi.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/vhost-user-blk/vhost-user-blk.c b/contrib/vhost-user-blk/vhost-user-blk.c
index 25eccd02b5..40a2dfc544 100644
--- a/contrib/vhost-user-blk/vhost-user-blk.c
+++ b/contrib/vhost-user-blk/vhost-user-blk.c
@@ -474,7 +474,7 @@ static int unix_sock_new(char *unix_fn)
     assert(unix_fn);

     sock = socket(AF_UNIX, SOCK_STREAM, 0);
-    if (sock <= 0) {
+    if (sock < 0) {
         perror("socket");
         return -1;
     }
diff --git a/contrib/vhost-user-scsi/vhost-user-scsi.c b/contrib/vhost-user-scsi/vhost-user-scsi.c
index 3c912384e9..0f9ba4b2a2 100644
--- a/contrib/vhost-user-scsi/vhost-user-scsi.c
+++ b/contrib/vhost-user-scsi/vhost-user-scsi.c
@@ -320,7 +320,7 @@ static int unix_sock_new(char *unix_fn)
     assert(unix_fn);

     sock = socket(AF_UNIX, SOCK_STREAM, 0);
-    if (sock <= 0) {
+    if (sock < 0) {
         perror("socket");
         return -1;
     }
-- 
2.19.1
Re: [PATCH V2] vhost-user-blk/scsi: Fix broken error handling for socket call
Posted by Raphael Norwitz 5 years, 3 months ago 
On Thu, Oct 29, 2020 at 2:04 AM AlexChen <alex.chen@huawei.com> wrote:
>
> When socket() fails, it returns -1, 0 is the normal return value and should not return error.
>
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: AlexChen <alex.chen@huawei.com>

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


> ---
>  contrib/vhost-user-blk/vhost-user-blk.c   | 2 +-
>  contrib/vhost-user-scsi/vhost-user-scsi.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/vhost-user-blk/vhost-user-blk.c b/contrib/vhost-user-blk/vhost-user-blk.c
> index 25eccd02b5..40a2dfc544 100644
> --- a/contrib/vhost-user-blk/vhost-user-blk.c
> +++ b/contrib/vhost-user-blk/vhost-user-blk.c
> @@ -474,7 +474,7 @@ static int unix_sock_new(char *unix_fn)
>      assert(unix_fn);
>
>      sock = socket(AF_UNIX, SOCK_STREAM, 0);
> -    if (sock <= 0) {
> +    if (sock < 0) {
>          perror("socket");
>          return -1;
>      }
> diff --git a/contrib/vhost-user-scsi/vhost-user-scsi.c b/contrib/vhost-user-scsi/vhost-user-scsi.c
> index 3c912384e9..0f9ba4b2a2 100644
> --- a/contrib/vhost-user-scsi/vhost-user-scsi.c
> +++ b/contrib/vhost-user-scsi/vhost-user-scsi.c
> @@ -320,7 +320,7 @@ static int unix_sock_new(char *unix_fn)
>      assert(unix_fn);
>
>      sock = socket(AF_UNIX, SOCK_STREAM, 0);
> -    if (sock <= 0) {
> +    if (sock < 0) {
>          perror("socket");
>          return -1;
>      }
> --
> 2.19.1
>
Re: [PATCH V2] vhost-user-blk/scsi: Fix broken error handling for socket call
Posted by Peter Maydell 5 years, 2 months ago 
Ping? This is a coverity issue fix which has been reviewed,
whose tree should it go via?

Adding mst to cc list as the listed maintainer.

thanks
-- PMM

On Thu, 29 Oct 2020 at 06:05, AlexChen <alex.chen@huawei.com> wrote:
>
> When socket() fails, it returns -1, 0 is the normal return value and should not return error.
>
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: AlexChen <alex.chen@huawei.com>
> ---
>  contrib/vhost-user-blk/vhost-user-blk.c   | 2 +-
>  contrib/vhost-user-scsi/vhost-user-scsi.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/vhost-user-blk/vhost-user-blk.c b/contrib/vhost-user-blk/vhost-user-blk.c
> index 25eccd02b5..40a2dfc544 100644
> --- a/contrib/vhost-user-blk/vhost-user-blk.c
> +++ b/contrib/vhost-user-blk/vhost-user-blk.c
> @@ -474,7 +474,7 @@ static int unix_sock_new(char *unix_fn)
>      assert(unix_fn);
>
>      sock = socket(AF_UNIX, SOCK_STREAM, 0);
> -    if (sock <= 0) {
> +    if (sock < 0) {
>          perror("socket");
>          return -1;
>      }
> diff --git a/contrib/vhost-user-scsi/vhost-user-scsi.c b/contrib/vhost-user-scsi/vhost-user-scsi.c
> index 3c912384e9..0f9ba4b2a2 100644
> --- a/contrib/vhost-user-scsi/vhost-user-scsi.c
> +++ b/contrib/vhost-user-scsi/vhost-user-scsi.c
> @@ -320,7 +320,7 @@ static int unix_sock_new(char *unix_fn)
>      assert(unix_fn);
>
>      sock = socket(AF_UNIX, SOCK_STREAM, 0);
> -    if (sock <= 0) {
> +    if (sock < 0) {
>          perror("socket");
>          return -1;
>      }
> --
> 2.19.1
Re: [PATCH V2] vhost-user-blk/scsi: Fix broken error handling for socket call
Posted by Michael S. Tsirkin 5 years, 2 months ago 
On Thu, Oct 29, 2020 at 02:03:52PM +0800, AlexChen wrote:
> When socket() fails, it returns -1, 0 is the normal return value and should not return error.
> 
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: AlexChen <alex.chen@huawei.com>

Tagged, thanks!

> ---
>  contrib/vhost-user-blk/vhost-user-blk.c   | 2 +-
>  contrib/vhost-user-scsi/vhost-user-scsi.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/contrib/vhost-user-blk/vhost-user-blk.c b/contrib/vhost-user-blk/vhost-user-blk.c
> index 25eccd02b5..40a2dfc544 100644
> --- a/contrib/vhost-user-blk/vhost-user-blk.c
> +++ b/contrib/vhost-user-blk/vhost-user-blk.c
> @@ -474,7 +474,7 @@ static int unix_sock_new(char *unix_fn)
>      assert(unix_fn);
> 
>      sock = socket(AF_UNIX, SOCK_STREAM, 0);
> -    if (sock <= 0) {
> +    if (sock < 0) {
>          perror("socket");
>          return -1;
>      }
> diff --git a/contrib/vhost-user-scsi/vhost-user-scsi.c b/contrib/vhost-user-scsi/vhost-user-scsi.c
> index 3c912384e9..0f9ba4b2a2 100644
> --- a/contrib/vhost-user-scsi/vhost-user-scsi.c
> +++ b/contrib/vhost-user-scsi/vhost-user-scsi.c
> @@ -320,7 +320,7 @@ static int unix_sock_new(char *unix_fn)
>      assert(unix_fn);
> 
>      sock = socket(AF_UNIX, SOCK_STREAM, 0);
> -    if (sock <= 0) {
> +    if (sock < 0) {
>          perror("socket");
>          return -1;
>      }
> -- 
> 2.19.1
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.