This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] [BZ 17542] sunrpc: conditional jump depends on uninitialised value in svc_getreq_common


On 03/02/2015 04:34 AM, Siddhesh Poyarekar wrote:
> On Mon, Mar 02, 2015 at 10:29:31AM +0100, Andreas Schwab wrote:
>> Siddhesh Poyarekar <siddhesh@redhat.com> writes:
>>
>>> Andreas, is this OK or are you still not convinced about adding this
>>> patch in?
>>
>> How about fixing the real bug?
> 
> Carlos has posted a fix for nfs-utils[1].  The intent of including
> this patch is not to work around that specific bug, but to make the
> code robust against such bugs in future.
> 
> Siddhesh
> 
> [1] http://article.gmane.org/gmane.linux.nfs/69437
> 

This should be committed to glibc.

This is now committed to nfs-utils. I'd say we should make SunRPC
robust also, just as a matter of fact. We still have old applications
using the compatibility symbols that can benefit from having unregistered
fds be ignored.

commit 810423415dd1a2b7275b3abf294e6a69951614a1
Author: Carlos O'Donell <carlos@redhat.com>
Date:   Thu Feb 26 14:13:26 2015 -0500

    rpc.statd: Avoid passing unregistered socket to svc_getreqset
    
    rpc.statd may crash if it receives both a notification reply and a
    client connection at the same time. It crashes because it adds
    sockfd to SVC_FDSET and that violates the API contract.
    
    The SVC_FDSET is to be considered read-only and must not be modified
    by user code. The daemon modifies it for expediency to avoid
    having to maintain two distinct fd lists and select on each one.
    It is a practical choice that makes sense.
    
    Thus, if a notification reply arrives by itself everything works,
    or if a client connection arrives by itself everything works. Both
    must arrive at the same time for sockfd to be set in SVC_FDSET
    and to be processed by svc_getreqset because more than one of
    readfds is ready.
    
    It is the processing by svc_getreqset that will crash when it finds an
    unregistered fd in the list that doesn't correlate to any of the
    internal book keeping done by the library. At present the glibc
    SunRPC library will crash, but TIRPC does not (it is robust against
    invalid API usage in this case). However, future RPC libraries
    may be implemented differently, and the questionable API usage
    should be fixed.
    
    The simplest fix is for process_reply to *clear* sockfd from the
    ready-to-read fds, since it was never registered with xprt_register.
    This works because the code always calls process_reply before handing
    the fd set to the RPC layer for processing.
    
    Compile-tested on x86_64 against master.
    
    Signed-off-by: Carlos O'Donell <carlos@redhat.com>
    Signed-off-by: Steve Dickson <steved@redhat.com>

Cheers,
Carlos.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]