From: Peter Maydell <peter.maydell@linaro.org>
To: qemu-devel@nongnu.org
Cc: patches@linaro.org,
Samuel Thibault <samuel.thibault@ens-lyon.org>,
Jan Kiszka <jan.kiszka@siemens.com>
Subject: [Qemu-devel] [PATCH for-3.1 0/4] slirp: fix coverity issues
Date: Tue, 6 Nov 2018 15:13:19 +0000 [thread overview]
Message-ID: <20181106151323.16154-1-peter.maydell@linaro.org> (raw)
There are three outstanding Coverity issues for the slirp code.
This patchset fixes them:
* easiest first, we explicitly check for so->s == -1 in slirp_send()
to avoid possibly passing -1 to the send() function.
In an ideal world we could assert() it wasn't -1, but the
slirp code is too complicated to be sure...
* we render a bug in an error-handling codepath moot by
switching socreate() to use g_new() rather than malloc(),
which means it can no longer fail and we can delete all the
error-handling code
* last and most complicated, we fix a lack of error handling
and possible deadlock in fork_exec() by moving the child
socket creation and connect to before fork(), where we can
handle the errors appropriately
Tested with an aarch64 Linux guest which I got to do various
networky things including an 'apt-get update' and downloading
some packages. Also tested the '-net user,smb=/some/dir' which
is the most common use of the fork_exec() codepath.
I've taken the usual approach of "fix up the indentation
only locally so as to placate checkpatch" with this patchset.
Happy to adjust if you have some other preference.
(I'm veering towards the opinion that at some point we should
just say "globally reindent slirp/, because the piecemeal
approach hasn't in practice resulted in a very good outcome";
but that's up to you.)
Peter Maydell (4):
slirp: Don't pass possibly -1 fd to send()
slirp: Use g_new() to allocate sockets in socreate()
slirp: Remove code that handles socreate() failure
slirp: fork_exec(): create and connect child socket before fork()
slirp/ip_icmp.c | 2 +-
slirp/misc.c | 55 +++++++++++++++++++++++++++++------------------
slirp/slirp.c | 14 +++++++++---
slirp/socket.c | 17 ++++++---------
slirp/tcp_input.c | 7 +++---
slirp/tcp_subr.c | 7 +-----
slirp/udp.c | 6 ------
slirp/udp6.c | 3 ---
8 files changed, 56 insertions(+), 55 deletions(-)
--
2.19.1
next reply other threads:[~2018-11-06 15:28 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-06 15:13 Peter Maydell [this message]
2018-11-06 15:13 ` [Qemu-devel] [PATCH for-3.1 1/4] slirp: Don't pass possibly -1 fd to send() Peter Maydell
2018-11-06 23:05 ` Samuel Thibault
2018-11-06 15:13 ` [Qemu-devel] [PATCH for-3.1 2/4] slirp: Use g_new() to allocate sockets in socreate() Peter Maydell
2018-11-06 23:07 ` Samuel Thibault
2018-11-06 15:13 ` [Qemu-devel] [PATCH for-3.1 3/4] slirp: Remove code that handles socreate() failure Peter Maydell
2018-11-06 23:08 ` Samuel Thibault
2018-11-06 15:13 ` [Qemu-devel] [PATCH for-3.1 4/4] slirp: fork_exec(): create and connect child socket before fork() Peter Maydell
2018-11-06 23:11 ` Samuel Thibault
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20181106151323.16154-1-peter.maydell@linaro.org \
--to=peter.maydell@linaro.org \
--cc=jan.kiszka@siemens.com \
--cc=patches@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=samuel.thibault@ens-lyon.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).