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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20180301123118.GB28739@oracle.com>
Date:   Thu, 1 Mar 2018 07:31:18 -0500
From:   Sowmini Varadhan <sowmini.varadhan@...cle.com>
To:     Ka-Cheong Poon <ka-cheong.poon@...cle.com>
Cc:     Sowmini Varadhan <sowmini05@...il.com>,
        netdev <netdev@...r.kernel.org>, santosh.shilimkar@...cle.com,
        David Miller <davem@...emloft.net>, rds-devel@....oracle.com
Subject: Re: [PATCH net] rds: Incorrect reference counting in TCP socket
 creation

On (03/01/18 20:19), Ka-Cheong Poon wrote:
> >>
> >>-       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.

I see, thanks for clarification.

It may be helpful to have some comment in there, in case some other
module trips on something similar in the future.

--Sowmini