[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] qemu-nbd: Fix coverity issues
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH] qemu-nbd: Fix coverity issues |
Date: |
Mon, 17 Mar 2014 09:49:56 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.2 (gnu/linux) |
Paolo Bonzini <address@hidden> writes:
> There are two issues in qemu-nbd: a missing return value check after
> calling accept(), and file descriptor leaks in nbd_client_thread.
>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
> qemu-nbd.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index bdac1f3..899e67c 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -288,19 +288,19 @@ static void *nbd_client_thread(void *arg)
> ret = nbd_receive_negotiate(sock, NULL, &nbdflags,
> &size, &blocksize);
> if (ret < 0) {
> - goto out;
> + goto out_socket;
> }
>
> fd = open(device, O_RDWR);
> if (fd < 0) {
> /* Linux-only, we can use %m in printf. */
> fprintf(stderr, "Failed to open %s: %m", device);
> - goto out;
> + goto out_socket;
> }
>
> ret = nbd_init(fd, sock, nbdflags, size, blocksize);
> if (ret < 0) {
> - goto out;
> + goto out_fd;
> }
>
> /* update partition table */
> @@ -316,12 +316,16 @@ static void *nbd_client_thread(void *arg)
>
> ret = nbd_client(fd);
> if (ret) {
> - goto out;
> + goto out_fd;
> }
> close(fd);
> kill(getpid(), SIGTERM);
> return (void *) EXIT_SUCCESS;
>
> +out_fd:
> + close(fd);
> +out_socket:
> + closesocket(sock);
> out:
> kill(getpid(), SIGTERM);
> return (void *) EXIT_FAILURE;
The return values are disgusting, but that's not your fault. Hmm,
actually it is: commit a517e88b.
> @@ -355,6 +359,11 @@ static void nbd_accept(void *opaque)
> socklen_t addr_len = sizeof(addr);
>
> int fd = accept(server_fd, (struct sockaddr *)&addr, &addr_len);
> + if (fd < 0) {
> + perror("accept");
> + return;
> + }
> +
> if (state >= TERMINATE) {
> close(fd);
> return;
Reviewed-by: Markus Armbruster <address@hidden>