netdev - Re: [PATCH bpf-next 01/14] net: Change sock_setsockopt from taking sock ptr to sk ptr

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: <20220727204206.fhiamb66c3sljhg4@kafai-mbp.dhcp.thefacebook.com>
Date:   Wed, 27 Jul 2022 13:42:06 -0700
From:   Martin KaFai Lau <kafai@...com>
To:     David Laight <David.Laight@...lab.com>
Cc:     "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Andrii Nakryiko <andrii@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        David Miller <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        "kernel-team@...com" <kernel-team@...com>,
        Paolo Abeni <pabeni@...hat.com>
Subject: Re: [PATCH bpf-next 01/14] net: Change sock_setsockopt from taking
 sock ptr to sk ptr

On Wed, Jul 27, 2022 at 08:11:26AM +0000, David Laight wrote:
> From: Martin KaFai Lau
> > Sent: 27 July 2022 07:09
> > 
> > A latter patch refactors bpf_setsockopt(SOL_SOCKET) with the
> > sock_setsockopt() to avoid code duplication and code
> > drift between the two duplicates.
> > 
> > The current sock_setsockopt() takes sock ptr as the argument.
> > The very first thing of this function is to get back the sk ptr
> > by 'sk = sock->sk'.
> > 
> > bpf_setsockopt() could be called when the sk does not have
> > a userspace owner.  Meaning sk->sk_socket is NULL.  For example,
> > when a passive tcp connection has just been established.  Thus,
> > it cannot use the sock_setsockopt(sk->sk_socket) or else it will
> > pass a NULL sock ptr.
> 
> I'm intrigued, I've some code that uses sock_create_kern() to create
> sockets without a userspace owner - I'd have though bpf is doing
> much the same.
> 
> I end up doing:
>         if (level == SOL_SOCKET)
>                 err = sock_setsockopt(sock, level, optname, koptval, optlen);
>         else
>                 err = sock->ops->setsockopt(sock, level, optname, koptval,
>                                             optlen);
> to set options.
> (This code used to use kern_setsockopt() - but that got removed.)
> 
> I'd have though bpf would need similar code??
By no userspace owner, I was referring a sk has not been accept()-ed by
the userspace yet instead of a 'struct socket *sock' created for
the kernel internal use.  After another thought, that tcp_sock
is sort of owned by the listen sk,  I will rephrase the commit
message to avoid the confusion.

bpf prog does not have a 'sock' ptr because the sock
has not been created yet.