netdev - Re: [PATCH v8 1/4] net/handshake: Create a NETLINK service for handling handshake requests

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: <78F3B3C0-D863-4DC6-BAA0-9730ECE32529@oracle.com>
Date:   Wed, 5 Apr 2023 14:10:19 +0000
From:   Chuck Lever III <chuck.lever@...cle.com>
To:     Hannes Reinecke <hare@...e.de>
CC:     Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
        Eric Dumazet <edumazet@...gle.com>,
        Boris Pismenny <borisp@...dia.com>,
        "open list:NETWORKING [GENERAL]" <netdev@...r.kernel.org>,
        "kernel-tls-handshake@...ts.linux.dev" 
        <kernel-tls-handshake@...ts.linux.dev>,
        John Haxby <john.haxby@...cle.com>
Subject: Re: [PATCH v8 1/4] net/handshake: Create a NETLINK service for
 handling handshake requests



> On Apr 5, 2023, at 2:32 AM, Hannes Reinecke <hare@...e.de> wrote:
> 
> On 4/5/23 02:00, Jakub Kicinski wrote:
>> On Tue, 4 Apr 2023 17:44:19 +0200 Hannes Reinecke wrote:
>>>> We're still seeing NULL pointer dereferences here.
>>>> Typically this happens after the remote closes the
>>>> connection early.
>>>> 
>>>> I guess I cannot rely on sock_hold(sk); from preventing
>>>> someone from doing a "sock->sk = NULL;"
>>>> 
>>>> Would it make more sense for req_submit and req_cancel to
>>>> operate on "struct sock *" rather than "struct socket *" ?
>>>>   
>>> Stumbled across that one, too; that's why my initial submission
>>> was sprinkled with 'if (!sock->sk)' statements.
>>> So I think it's a good idea.
>>> 
>>> But waiting for Jakub to enlighten us.
>> Ah, I'm probably the weakest of the netdev maintainers when it comes
>> to the socket layer :)
>> I thought sock->sk is only cleared if the "user" of the socket closes
>> it. But yes, both sock->sk == NULL and sk->sk_socket == NULL are
>> entirely possible, and the networking stack usually operates on
>> struct sock. Why exactly those two are separate beings is one of
>> the mysteries of Linux networking which causes guaranteed confusion
>> to all the newcomers. I wish I knew the details so I could at least
>> document it :S
> 
> Bummer. I had high hopes on you being able to shed some light on this.
> 
> So, Chuck: maybe we should be looking at switching over to 'struct sock' for the internal stuff. If we don't have to do a 'fput()' somewhere we should be good...

I've made handshake_req_cancel() take a "struct sock *" as
a starting point. I'll send out something for you to try
later today.


--
Chuck Lever