Re: [v2 PATCH for-4.4 04/16] SUNRPC: fix refcounting problems with auth_gss messages.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Apr 12, 2017 at 11:13:38PM +0530, Sumit Semwal wrote:
> From: NeilBrown <neilb@xxxxxxxx>
> 
> [ Upstream commit 1cded9d2974fe4fe339fc0ccd6638b80d465ab2c ]
> 
> 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: 9130b8dbc6ac ("SUNRPC: allow for upcalls for same uid but different gss service")
> Cc: stable@xxxxxxxxxxxxxxx
> Link: https://bugzilla.opensuse.org/show_bug.cgi?id=1011250
> Signed-off-by: NeilBrown <neilb@xxxxxxxx>
> Signed-off-by: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
> Signed-off-by: Sumit Semwal <sumit.semwal@xxxxxxxxxx>
> ---
>  net/sunrpc/auth_gss/auth_gss.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

This patch is already in 4.9.2!  Something went really wrong with your
patch selection here, why are you sending patches I already have?

confused,

greg k-h



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]