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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230404170035.6650027d@kernel.org>
Date:   Tue, 4 Apr 2023 17:00:35 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Hannes Reinecke <hare@...e.de>
Cc:     Chuck Lever III <chuck.lever@...cle.com>,
        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 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