On 20/05/2018 08:15, Michael Tokarev wrote:
> From: Peter Maydell <peter.mayd...@linaro.org>
>
> In gdb_accept(), we both fail to check all errors (notably
> that from socket_set_nodelay(), as Coverity notes in CID 1005666),
> and fail to return an error status back to our caller. Correct
> both of these things, so that errors in accept() result in our
> stopping with a useful error message rather than ignoring it.
>
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
> Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org>
> Reviewed-by: Thomas Huth <th...@redhat.com>
> Signed-off-by: Michael Tokarev <m...@tls.msk.ru>
> ---
> gdbstub.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index b99980d2e2..e4ece2f5bc 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1814,7 +1814,7 @@ void gdb_signalled(CPUArchState *env, int sig)
> put_packet(s, buf);
> }
>
> -static void gdb_accept(void)
> +static bool gdb_accept(void)
> {
> GDBState *s;
> struct sockaddr_in sockaddr;
> @@ -1826,7 +1826,7 @@ static void gdb_accept(void)
> fd = accept(gdbserver_fd, (struct sockaddr *)&sockaddr, &len);
> if (fd < 0 && errno != EINTR) {
> perror("accept");
> - return;
> + return false;
> } else if (fd >= 0) {
> qemu_set_cloexec(fd);
> break;
> @@ -1834,7 +1834,10 @@ static void gdb_accept(void)
> }
>
> /* set short latency */
> - socket_set_nodelay(fd);
> + if (socket_set_nodelay(fd)) {
> + perror("setsockopt");
> + return false;
Coverity notes that this leaks fd.
Paolo
> + }
>
> s = g_malloc0(sizeof(GDBState));
> s->c_cpu = first_cpu;
> @@ -1843,6 +1846,7 @@ static void gdb_accept(void)
> gdb_has_xml = false;
>
> gdbserver_state = s;
> + return true;
> }
>
> static int gdbserver_open(int port)
> @@ -1883,7 +1887,11 @@ int gdbserver_start(int port)
> if (gdbserver_fd < 0)
> return -1;
> /* accept connections */
> - gdb_accept();
> + if (!gdb_accept()) {
> + close(gdbserver_fd);
> + gdbserver_fd = -1;
> + return -1;
> + }
> return 0;
> }
>
>
