[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Qemu-trivial] [PATCH] virtio-9p-proxy: Fix sockfd leak
From: |
Michael Tokarev |
Subject: |
Re: [Qemu-devel] [Qemu-trivial] [PATCH] virtio-9p-proxy: Fix sockfd leak and modify the check logic |
Date: |
Thu, 30 Oct 2014 11:03:24 +0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Icedove/24.8.1 |
29.10.2014 13:52, address@hidden wrote:
> From: Gonglei <address@hidden>
>
> If connect() return false, the sockfd will leak,
> meanwhile proxy_init() can't check the return value
> of connect_namedsocket(), maybe cause unpredictable
> results.
>
> Let's move the sock_id check logic out, which can
> check both if and else statements.
>
> Signed-off-by: Gonglei <address@hidden>
> ---
> hw/9pfs/virtio-9p-proxy.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/hw/9pfs/virtio-9p-proxy.c b/hw/9pfs/virtio-9p-proxy.c
> index b57966d..1c3aa5f 100644
> --- a/hw/9pfs/virtio-9p-proxy.c
> +++ b/hw/9pfs/virtio-9p-proxy.c
> @@ -1112,6 +1112,7 @@ static int connect_namedsocket(const char *path)
> size = strlen(helper.sun_path) + sizeof(helper.sun_family);
> if (connect(sockfd, (struct sockaddr *)&helper, size) < 0) {
> fprintf(stderr, "socket error\n");
> + close(sockfd);
> return -1;
> }
>
> @@ -1152,11 +1153,12 @@ static int proxy_init(FsContext *ctx)
> sock_id = connect_namedsocket(ctx->fs_root);
> } else {
> sock_id = atoi(ctx->fs_root);
> - if (sock_id < 0) {
> - fprintf(stderr, "socket descriptor not initialized\n");
> - g_free(proxy);
> - return -1;
> - }
> + }
> +
> + if (sock_id < 0) {
> + fprintf(stderr, "socket descriptor not initialized\n");
> + g_free(proxy);
> + return -1;
> }
> g_free(ctx->fs_root);
> ctx->fs_root = NULL;
Um. I'm applying 2 patches instead of this one. First:
virtio-9p-proxy: Fix sockfd leak
If connect() in connect_namedsocket() return false, the sockfd will leak.
Plug it.
Signed-off-by: Michael Tokarev <address@hidden>
Signed-off-by: Gonglei <address@hidden>
diff --git a/hw/9pfs/virtio-9p-proxy.c b/hw/9pfs/virtio-9p-proxy.c
index b57966d..e6bbb06 100644
--- a/hw/9pfs/virtio-9p-proxy.c
+++ b/hw/9pfs/virtio-9p-proxy.c
@@ -1112,6 +1112,7 @@ static int connect_namedsocket(const char *path)
size = strlen(helper.sun_path) + sizeof(helper.sun_family);
if (connect(sockfd, (struct sockaddr *)&helper, size) < 0) {
fprintf(stderr, "socket error\n");
+ close(sockfd);
return -1;
}
And second. Note the slight change in logic and error messages
compared with your version - there's no need to print error
message twice if connect_namedsocket() returned -1 (it already
printed error message).
virtio-9p-proxy: fix error return in proxy_init()
proxy_init() does not check the return value of connect_namedsocket(),
fix this by rearranging code a little bit.
Signed-off-by: Michael Tokarev <address@hidden>
diff --git a/hw/9pfs/virtio-9p-proxy.c b/hw/9pfs/virtio-9p-proxy.c
index e6bbb06..2ec211b 100644
--- a/hw/9pfs/virtio-9p-proxy.c
+++ b/hw/9pfs/virtio-9p-proxy.c
@@ -1155,10 +1155,12 @@ static int proxy_init(FsContext *ctx)
sock_id = atoi(ctx->fs_root);
if (sock_id < 0) {
fprintf(stderr, "socket descriptor not initialized\n");
- g_free(proxy);
- return -1;
}
}
+ if (sock_id < 0) {
+ g_free(proxy);
+ return -1;
+ }
g_free(ctx->fs_root);
ctx->fs_root = NULL;
And I'll immediately send another followup patch to improve
error messages in connect_namedsocket(), -- these are awful.
/mjt