Re: [PATCH net] rds: Incorrect reference counting in TCP socket creation

[Ka-Cheong Poon <ka-cheong.poon@oracle.com>]

On 03/01/2018 07:54 PM, Sowmini Varadhan wrote:
> On Wed, Feb 28, 2018 at 11:44 PM, Ka-Cheong Poon
> <ka-cheong.poon@oracle.com> wrote:
>> Commit 0933a578cd55 ("rds: tcp: use sock_create_lite() to create the
>> accept socket") has a reference counting issue in TCP socket creation
>> when accepting a new connection.  The code uses sock_create_lite() to
>> create a kernel socket.  But it does not do __module_get() on the
>> socket owner.  When the connection is shutdown and sock_release() is
>> called to free the socket, the owner's reference count is decremented
>> and becomes incorrect.  Note that this bug only shows up when the socket
>> owner is configured as a kernel module.
> 
>>
>> -       new_sock->type = sock->type;
>> -       new_sock->ops = sock->ops;
>>          ret = sock->ops->accept(sock, new_sock, O_NONBLOCK, true);
>>          if (ret < 0)
>>                  goto out;
>>
>> +       new_sock->ops = sock->ops;
> 
> How is this delta relevant to the commit comment? Seems unrelated?


Note that sock_release() checks if sock->ops is set before
decrementing the refcnt.  By moving the ops assignment after
the ops->accept() call, we save increasing the refcnt in
case the ops->accept() fails.  Otherwise, the __module_get()
needs to be moved before ops->accept() to handle this failure
case.


-- 
K. Poon
ka-cheong.poon@oracle.com