• NeilBrown's avatar
    SUNRPC: fix refcounting problems with auth_gss messages. · 8dc821b9
    NeilBrown authored
    commit 1cded9d2 upstream.
    
    There are two problems with refcounting of auth_gss messages.
    
    First, the reference on the pipe->pipe list (taken by a call
    to rpc_queue_upcall()) is not counted.  It seems to be
    assumed that a message in pipe->pipe will always also be in
    pipe->in_downcall, where it is correctly reference counted.
    
    However there is no guaranty of this.  I have a report of a
    NULL dereferences in rpc_pipe_read() which suggests a msg
    that has been freed is still on the pipe->pipe list.
    
    One way I imagine this might happen is:
    - message is queued for uid=U and auth->service=S1
    - rpc.gssd reads this message and starts processing.
      This removes the message from pipe->pipe
    - message is queued for uid=U and auth->service=S2
    - rpc.gssd replies to the first message. gss_pipe_downcall()
      calls __gss_find_upcall(pipe, U, NULL) and it finds the
      *second* message, as new messages are placed at the head
      of ->in_downcall, and the service type is not checked.
    - This second message is removed from ->in_downcall and freed
      by gss_release_msg() (even though it is still on pipe->pipe)
    - rpc.gssd tries to read another message, and dereferences a pointer
      to this message that has just been freed.
    
    I fix this by incrementing the reference count before calling
    rpc_queue_upcall(), and decrementing it if that fails, or normally in
    gss_pipe_destroy_msg().
    
    It seems strange that the reply doesn't target the message more
    precisely, but I don't know all the details.  In any case, I think the
    reference counting irregularity became a measureable bug when the
    extra arg was added to __gss_find_upcall(), hence the Fixes: line
    below.
    
    The second problem is that if rpc_queue_upcall() fails, the new
    message is not freed. gss_alloc_msg() set the ->count to 1,
    gss_add_msg() increments this to 2, gss_unhash_msg() decrements to 1,
    then the pointer is discarded so the memory never gets freed.
    
    Fixes: 9130b8db ("SUNRPC: allow for upcalls for same uid but different gss service")
    Link: https://bugzilla.opensuse.org/show_bug.cgi?id=1011250Signed-off-by: default avatarNeilBrown <neilb@suse.com>
    Signed-off-by: default avatarTrond Myklebust <trond.myklebust@primarydata.com>
    Signed-off-by: default avatarSumit Semwal <sumit.semwal@linaro.org>
    Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
    8dc821b9
auth_gss.c 53.1 KB