| Message ID | 5451F0CC.3030100@msgid.tls.msk.ru |
|---|---|
| State | New |
| Headers | show |
On 2014/10/30 16:03, Michael Tokarev wrote: > 29.10.2014 13:52, arei.gonglei@huawei.com wrote: >> From: Gonglei <arei.gonglei@huawei.com> >> >> 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 <arei.gonglei@huawei.com> >> --- >> 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: > > Ok, Thanks for your work. Best regards, -Gonglei > > virtio-9p-proxy: Fix sockfd leak > > If connect() in connect_namedsocket() return false, the sockfd will leak. > Plug it. > > Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> > Signed-off-by: Gonglei <arei.gonglei@huawei.com> > > 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 <mjt@tls.msk.ru> > > 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
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 <mjt@tls.msk.ru> 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;