On 05/24/2018 06:35 PM, Paolo Bonzini wrote: > 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.
Oops didn't noticed. It this is so trivial than Coverity could directly send the fix to the list, like modern compilers. >> + } >> >> 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; >> }