* Peter Maydell (peter.mayd...@linaro.org) wrote:
> In a fork_exec() error path we try to closesocket(s) when s might
> be a negative number because the thing that failed was the
> qemu_socket() call. Add a guard so we don't do this.
>
> (Spotted by Coverity: CID 1005727 issue 1 of 2.)
>
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
> ---
> Issue 2 of 2 in CID 1005727 is trickier -- we need to move as
> much as possible of the client-end connect/accept out of the
> child process and into the parent as possible. I'm not sure
> if it's safe to do it all in the parent without deadlocking...
or just bail earlier? The bit that worries me there
is the dup2(s, [012]); which is called unchecked, if that fails
then your telnetd or whatever probably ends up connected to whatever
your 0..2 were originally.
> slirp/misc.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/slirp/misc.c b/slirp/misc.c
> index 88e9d94197..260187b6b6 100644
> --- a/slirp/misc.c
> +++ b/slirp/misc.c
> @@ -112,7 +112,9 @@ fork_exec(struct socket *so, const char *ex, int do_pty)
> bind(s, (struct sockaddr *)&addr, addrlen) < 0 ||
> listen(s, 1) < 0) {
> error_report("Error: inet socket: %s", strerror(errno));
> - closesocket(s);
> + if (s >= 0) {
> + closesocket(s);
> + }
Reviewed-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
(I'm not convinced this would ever do anything bad, at least on a *nix
system, the -ve value is always going to be an invalid fd so the close
will just fail).
Dave
> return 0;
> }
> --
> 2.11.0
>
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
