[Bug libc/17542] New: conditional jump depends on uninitialised value in svc_getreq_common

[Bug libc/17542] New: conditional jump depends on uninitialised value in svc_getreq_common

[bhubbard at redhat dot com]

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

            Bug ID: 17542
           Summary: conditional jump depends on uninitialised value in
                    svc_getreq_common
           Product: glibc
           Version: 2.21
            Status: NEW
          Severity: normal
          Priority: P2
         Component: libc
          Assignee: unassigned at sourceware dot org
          Reporter: bhubbard at redhat dot com
                CC: drepper.fsp at gmail dot com

Created attachment 7888
  --> https://sourceware.org/bugzilla/attachment.cgi?id=7888&action=edit
Proposed patch to resolve uninitialised xports in xprt_register

If xports is NULL in xprt_register we malloc it but if sock > _rpc_dtablesize()
that memory does not get initialised and may in theory contain any value. Later
we make a conditional jump in svc_getreq_common based on the uninitialised
memory and this caused a general protection fault in rpc.statd on an older
version of glibc but this code has not changed since that version.

Following is the valgrind warning.

==26802== Conditional jump or move depends on uninitialised value(s)
==26802==    at 0x5343A25: svc_getreq_common (in /lib64/libc-2.5.so)
==26802==    by 0x534357B: svc_getreqset (in /lib64/libc-2.5.so)
==26802==    by 0x10DE1F: ??? (in /sbin/rpc.statd)
==26802==    by 0x10D0EF: main (in /sbin/rpc.statd)
==26802==  Uninitialised value was created by a heap allocation
==26802==    at 0x4C2210C: malloc (vg_replace_malloc.c:195)
==26802==    by 0x53438BE: xprt_register (in /lib64/libc-2.5.so)
==26802==    by 0x53450DF: svcudp_bufcreate (in /lib64/libc-2.5.so)
==26802==    by 0x10FE32: ??? (in /sbin/rpc.statd)
==26802==    by 0x10D13E: main (in /sbin/rpc.statd)

I believe the solution here is to change the malloc call to a calloc call and
the attached patch does that. the GPF could not be reproduced with the patched
glibc.

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

[Bug libc/17542] conditional jump depends on uninitialised value in svc_getreq_common

[bhubbard at redhat dot com]

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

Brad Hubbard <bhubbard at redhat dot com> changed:

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

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

[Bug libc/17542] conditional jump depends on uninitialised value in svc_getreq_common

[schwab@linux-m68k.org]

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

--- Comment #1 from Andreas Schwab <schwab@linux-m68k.org> ---
I can't see how that can fix anything if the index is outside the bounds of
xports.

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

[Bug libc/17542] conditional jump depends on uninitialised value in svc_getreq_common

[carlos at redhat dot com]

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

Carlos O'Donell <carlos at redhat dot com> changed:

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

--- Comment #2 from Carlos O'Donell <carlos at redhat dot com> ---
(In reply to Andreas Schwab from comment #1)
> I can't see how that can fix anything if the index is outside the bounds of
> xports.

The failure itself only happens if xprt_register is not called to register the
service, in which case the entry in xports is uninitialized.

Granted this is technically an improper use of the API, but using calloc
hardens svc_getreq_common at the cost of using calloc.

I think we should switch to calloc to allow svc_getreq_common to detect
unregistered handles and ignore them.

All of this doesn't fix the failure mode that results from changing the process
fd limit after initializing xports. Fixing that would require checking if fd
was outside of xports[] at each service check, but that cost is in the hot
path, and I'd like to avoid it.

Just a reminder that sunrpc is legacy, and new applications should be using
TIRPC.

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

[Bug libc/17542] conditional jump depends on uninitialised value in svc_getreq_common

[cvs-commit at gcc dot gnu.org]

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

--- Comment #3 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, master has been updated
       via  ed6b0fe710b631b99ed9fc28cefedfe69a16dc55 (commit)
      from  f8aeae347377f3dfa8cbadde057adf1827fb1d44 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=ed6b0fe710b631b99ed9fc28cefedfe69a16dc55

commit ed6b0fe710b631b99ed9fc28cefedfe69a16dc55
Author: Brad Hubbard <bhubbard@redhat.com>
Date:   Wed Mar 18 14:51:26 2015 +0530

    Use calloc to allocate xports (BZ #17542)

    If xports is NULL in xprt_register we malloc it but if sock >
    _rpc_dtablesize() that memory does not get initialised and may in theory
    contain any value. Later we make a conditional jump in svc_getreq_common
    based on the uninitialised memory and this caused a general protection
    fault in rpc.statd on an older version of glibc but this code has not
    changed since that version.

    Following is the valgrind warning.

    ==26802== Conditional jump or move depends on uninitialised value(s)
    ==26802==    at 0x5343A25: svc_getreq_common (in /lib64/libc-2.5.so)
    ==26802==    by 0x534357B: svc_getreqset (in /lib64/libc-2.5.so)
    ==26802==    by 0x10DE1F: ??? (in /sbin/rpc.statd)
    ==26802==    by 0x10D0EF: main (in /sbin/rpc.statd)
    ==26802==  Uninitialised value was created by a heap allocation
    ==26802==    at 0x4C2210C: malloc (vg_replace_malloc.c:195)
    ==26802==    by 0x53438BE: xprt_register (in /lib64/libc-2.5.so)
    ==26802==    by 0x53450DF: svcudp_bufcreate (in /lib64/libc-2.5.so)
    ==26802==    by 0x10FE32: ??? (in /sbin/rpc.statd)
    ==26802==    by 0x10D13E: main (in /sbin/rpc.statd)

-----------------------------------------------------------------------

Summary of changes:
 ChangeLog    |    5 +++++
 NEWS         |   11 +++++------
 sunrpc/svc.c |    4 ++--
 3 files changed, 12 insertions(+), 8 deletions(-)

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

[Bug libc/17542] conditional jump depends on uninitialised value in svc_getreq_common

[siddhesh at redhat dot com]

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

Siddhesh Poyarekar <siddhesh at redhat dot com> changed:

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

--- Comment #4 from Siddhesh Poyarekar <siddhesh at redhat dot com> ---
Fixed in master.

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

[Bug libc/17542] conditional jump depends on uninitialised value in svc_getreq_common

[fweimer at redhat dot com]

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

Florian Weimer <fweimer at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fweimer at redhat dot com
              Flags|security-                   |security?

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