[PATCH for-6.2 1/4] net: Zero sockaddr_in in parse_host_port()

Peter Maydell posted 4 patches 4 years, 5 months ago
Maintainers: Thomas Huth <thuth@redhat.com>, Jason Wang <jasowang@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Laurent Vivier <lvivier@redhat.com>, "Philippe Mathieu-Daudé" <philmd@redhat.com>, Corey Minyard <minyard@acm.org>, "Alex Bennée" <alex.bennee@linaro.org>
[PATCH for-6.2 1/4] net: Zero sockaddr_in in parse_host_port()
Posted by Peter Maydell 4 years, 5 months ago
We don't currently zero-initialize the 'struct sockaddr_in' that
parse_host_port() fills in, so any fields we don't explicitly
initialize might be left as random garbage.  POSIX states that
implementations may define extensions in sockaddr_in, and that those
extensions must not trigger if zero-initialized.  So not zero
initializing might result in inadvertently triggering an impdef
extension.

memset() the sockaddr_in before we start to fill it in.

Fixes: Coverity CID 1005338
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 net/net.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/net.c b/net/net.c
index 76bbb7c31b4..52c99196c69 100644
--- a/net/net.c
+++ b/net/net.c
@@ -75,6 +75,8 @@ int parse_host_port(struct sockaddr_in *saddr, const char *str,
     const char *addr, *p, *r;
     int port, ret = 0;
 
+    memset(saddr, 0, sizeof(*saddr));
+
     substrings = g_strsplit(str, ":", 2);
     if (!substrings || !substrings[0] || !substrings[1]) {
         error_setg(errp, "host address '%s' doesn't contain ':' "
-- 
2.20.1


Re: [PATCH for-6.2 1/4] net: Zero sockaddr_in in parse_host_port()
Posted by Eric Blake 4 years, 5 months ago
On Fri, Aug 13, 2021 at 04:05:03PM +0100, Peter Maydell wrote:
> We don't currently zero-initialize the 'struct sockaddr_in' that
> parse_host_port() fills in, so any fields we don't explicitly
> initialize might be left as random garbage.  POSIX states that
> implementations may define extensions in sockaddr_in, and that those
> extensions must not trigger if zero-initialized.  So not zero
> initializing might result in inadvertently triggering an impdef
> extension.
> 
> memset() the sockaddr_in before we start to fill it in.

Technically, POSIX recommends default initialization, as in:

struct sockaddr_in sa = { 0 };
or:
static struct sockaddr_in sa_init;
struct sockaddr_in sa = sa_init;

because of odd platforms where default initialization compiles to
non-zero bits (think platforms where NULL and/or floating point 0.0 do
not have an all-zero-bit representation - yes, C is weird).  But in
practice, that does not plague any of the hardware qemu cares about,
so I'm just fine with memset.

> 
> Fixes: Coverity CID 1005338
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  net/net.c | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org