[PATCH] virsocketaddr: Zero @netmask in virSocketAddrPrefixToNetmask()
Laine Stump
laine at redhat.com
Sat Oct 10 16:39:59 UTC 2020
On 10/9/20 10:43 AM, Michal Privoznik wrote:
> The aim of virSocketAddrPrefixToNetmask() is to initialize passed
> virSocketAddr structure based on prefix length and family.
> However, it doesn't set all members in the struct which may lead
> to reads of uninitialized values:
>
> ==15421== Use of uninitialised value of size 8
> ==15421== at 0x50F297A: _itoa_word (in /lib64/libc-2.31.so)
> ==15421== by 0x510C8FE: __vfprintf_internal (in /lib64/libc-2.31.so)
> ==15421== by 0x5120295: __vsnprintf_internal (in /lib64/libc-2.31.so)
> ==15421== by 0x50F8969: snprintf (in /lib64/libc-2.31.so)
> ==15421== by 0x51BB602: getnameinfo (in /lib64/libc-2.31.so)
> ==15421== by 0x496DEE0: virSocketAddrFormatFull (virsocketaddr.c:486)
> ==15421== by 0x496DD9F: virSocketAddrFormat (virsocketaddr.c:444)
> ==15421== by 0x11871F: networkDnsmasqConfContents (bridge_driver.c:1404)
> ==15421== by 0x1118F5: testCompareXMLToConfFiles (networkxml2conftest.c:48)
> ==15421== by 0x111BAF: testCompareXMLToConfHelper (networkxml2conftest.c:112)
> ==15421== by 0x112679: virTestRun (testutils.c:142)
> ==15421== by 0x111D09: mymain (networkxml2conftest.c:144)
> ==15421== Uninitialised value was created by a stack allocation
> ==15421== at 0x1175D2: networkDnsmasqConfContents (bridge_driver.c:1056)
>
> All callers expect the function to initialize the structure
> fully.
>
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
Reviewed-by: Laine Stump <laine at redhat.com>
Did you see actual errors caused by this, or just the valgrind
complaints? It's been like this since the function was first implemented
in 2010 (commit 1ab80f32. Yes, it was me. Sigh.) I wonder why nobody
ever saw this valgrind complaint before. Some check that was newly added?
(unless you've seen a real-world error, my guess would be that the
fields that aren't being initialized are actually ignored when the
family is set to INET or INET6, but even then it's still safer to clear
everything out.)
> ---
> src/util/virsocketaddr.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c
> index e0eb76ded3..65aaa632c7 100644
> --- a/src/util/virsocketaddr.c
> +++ b/src/util/virsocketaddr.c
> @@ -1097,6 +1097,8 @@ virSocketAddrPrefixToNetmask(unsigned int prefix,
> virSocketAddrPtr netmask,
> int family)
> {
> + memset(netmask, 0, sizeof(*netmask));
> +
> netmask->data.stor.ss_family = AF_UNSPEC; /* assume failure */
>
> if (family == AF_INET) {
> @@ -1135,7 +1137,7 @@ virSocketAddrPrefixToNetmask(unsigned int prefix,
> }
>
> return 0;
> - }
> +}
>
> /**
> * virSocketAddrGetIPPrefix:
More information about the libvir-list
mailing list