[Bug network/14687] New: valgrind warning of uninitialised byte(s) in res_send.c

[Bug network/14687] New: valgrind warning of uninitialised byte(s) in res_send.c

[timothy.c.pepper at linux dot intel.com]

http://sourceware.org/bugzilla/show_bug.cgi?id=14687

             Bug #: 14687
           Summary: valgrind warning of uninitialised byte(s) in
                    res_send.c
           Product: glibc
           Version: 2.16
            Status: NEW
          Severity: normal
          Priority: P2
         Component: network
        AssignedTo: unassigned@sourceware.org
        ReportedBy: timothy.c.pepper@linux.intel.com
    Classification: Unclassified


Created attachment 6674
  --> http://sourceware.org/bugzilla/attachment.cgi?id=6674
Simple patch resolving the valgrind warning

glibc 2.16.0's commit c030f70c introduces some variables on the stack
which don't get fully initialized, leading to valgrind complaints in
the __libc_res_nsend() -> send_dg() -> sendmmsg() call chain, eg:

Syscall param sendmsg(mmsg[0].msg_hdr) points to uninitialised byte(s)
    at 0x5AEAC6B: sendmmsg (sendmmsg.c:36)
    by 0x97B4643: __libc_res_nsend (res_send.c:1133)
    by 0x97B1C66: __libc_res_nquery (res_query.c:225)
    by 0x97B28D9: __libc_res_nsearch (res_query.c:582)
    by 0x95A5AC7: _nss_dns_gethostbyname4_r (dns-host.c:313)
    by 0x5AD076A: gaih_inet (getaddrinfo.c:842)
    by 0x5AD4653: getaddrinfo (getaddrinfo.c:2421)
    by 0x55BF384: Curl_getaddrinfo_ex (in /usr/lib64/libcurl.so.4.2.0)
    by 0x55C61C9: getaddrinfo_thread (in /usr/lib64/libcurl.so.4.2.0)
    by 0x55C4BF5: curl_thread_create_thunk (in /usr/lib64/libcurl.so.4.2.0)
    by 0x57E1EBE: start_thread (pthread_create.c:308)
    by 0x5AE932C: clone (clone.S:114)
 Address 0x9393ff0 is on thread 5's stack
 Uninitialised value was created by a stack allocation
    at 0x97B39E0: __libc_res_nsend (res_send.c:347)

It looks like the iov[], reqs.msg_len and reqs.msg_hdr.msg_flags could be
uninitialized there.  A simple memset to zero resolves the warning and
seems sensible enough, but there may be more sensible values to which 
these members could be explicitly set by somebody who knows the code.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug network/14687] valgrind warning of uninitialised byte(s) in res_send.c

[timothy.c.pepper at linux dot intel.com]

http://sourceware.org/bugzilla/show_bug.cgi?id=14687

Tim Pepper <timothy.c.pepper at linux dot intel.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |timothy.c.pepper at linux
                   |                            |dot intel.com

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug network/14687] valgrind warning of uninitialised byte(s) in res_send.c

[siddhesh at redhat dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=14687

Siddhesh Poyarekar <siddhesh at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |siddhesh at redhat dot com
         Resolution|                            |INVALID

--- Comment #1 from Siddhesh Poyarekar <siddhesh at redhat dot com> 2012-10-23 09:17:24 UTC ---
It's a spurious warning by valgrind since relevant fields are initialized
correctly.  It does not make sense to add cruft just to make valgrind happy.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug network/14687] valgrind warning of uninitialised byte(s) in res_send.c

[timothy.c.pepper at linux dot intel.com]

http://sourceware.org/bugzilla/show_bug.cgi?id=14687

--- Comment #2 from Tim Pepper <timothy.c.pepper at linux dot intel.com> 2012-10-24 21:18:30 UTC ---
(In reply to comment #1)
> It's a spurious warning by valgrind since relevant fields are initialized
> correctly.  It does not make sense to add cruft just to make valgrind happy.

I can see that assessment.

But it is also a spurious warning that given the code path many many many
developers will have to see, worry about, spend time analyzing, and finally be
able to mark off their list of potential sources for whatever oddity they were
debugging once they've figured out what glibc is doing, and then they add a
valgrind suppression to their codebase.  That leads to a lot of extra work
glibc causes other developers and leads to cruft in their codebases.  Worst
case the glibc code evolves somehow to actually use the uninitialized fields
and rather than valgrind-using developers helping catch this, they've
suppressed it already at their end.  This is not positive for overall distro
supportability and maintenance.

If the memset is cruft, the fields unused in this code path are cruft.  Would
you find more acceptable the patch to differentiate the data structures used in
the send and receive paths, along with all the associated code changes to the
places that would need to use the new data structures?

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug network/14687] valgrind warning of uninitialised byte(s) in res_send.c

[siddhesh at redhat dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=14687

--- Comment #3 from Siddhesh Poyarekar <siddhesh at redhat dot com> 2012-10-25 02:01:37 UTC ---
(In reply to comment #2)
> glibc causes other developers and leads to cruft in their codebases.  Worst
> case the glibc code evolves somehow to actually use the uninitialized fields
> and rather than valgrind-using developers helping catch this, they've
> suppressed it already at their end.  This is not positive for overall distro
> supportability and maintenance.

In this particular case, something like this happening is unlikely because of
the specific way in which the code works. It is interface code between the
kernel and glibc, where the kernel initializes the uninitialized values. One
could argue that initializing these values would mask a kernel bug.

> If the memset is cruft, the fields unused in this code path are cruft.  Would
> you find more acceptable the patch to differentiate the data structures used in
> the send and receive paths, along with all the associated code changes to the
> places that would need to use the new data structures?

I personally find any addition of code to this path unacceptable because it is
unnecessary. If you are looking for consensus in the glibc community, then you
can post your patch on the libc-alpha mailing list. Please follow the
guidelines given in this link to post:

http://sourceware.org/glibc/wiki/Contribution%20checklist

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug network/14687] valgrind warning of uninitialised byte(s) in res_send.c

[law at redhat dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=14687

law at redhat dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |law at redhat dot com

--- Comment #4 from law at redhat dot com 2012-10-25 02:11:15 UTC ---
Tim,

Valgrind has a way for dealing with false positives of this nature;
specifically the suppressions need updating to handle this case.  This really
should be dealt with by the valgrind maintainers.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug network/14687] valgrind warning of uninitialised byte(s) in res_send.c

[brooks at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=14687

Brooks Moses <brooks at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |brooks at gcc dot gnu.org

--- Comment #5 from Brooks Moses <brooks at gcc dot gnu.org> ---
Actually, for the record it looks like this was considered a bug in valgrind,
on grounds that it's mishandling the syscall, and fixed there:
https://bugs.kde.org/show_bug.cgi?id=315441

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug network/14687] valgrind warning of uninitialised byte(s) in res_send.c

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=14687

Florian Weimer <fweimer at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |security-

-- 
You are receiving this mail because:
You are on the CC list for the bug.