RE: [PATCH net 2/2] sock: redo the psock vs ULP protection check

[John Fastabend <john.fastabend@gmail.com>]

Jakub Kicinski wrote:
> Commit 8a59f9d1e3d4 ("sock: Introduce sk->sk_prot->psock_update_sk_prot()")
> has moved the inet_csk_has_ulp(sk) check from sk_psock_init() to
> the new tcp_bpf_update_proto() function. I'm guessing that this
> was done to allow creating psocks for non-inet sockets.
> 
> Unfortunately the destruction path for psock includes the ULP
> unwind, so we need to fail the sk_psock_init() itself.
> Otherwise if ULP is already present we'll notice that later,
> and call tcp_update_ulp() with the sk_proto of the ULP
> itself, which will most likely result in the ULP looping
> its callbacks.
> 
> Fixes: 8a59f9d1e3d4 ("sock: Introduce sk->sk_prot->psock_update_sk_prot()")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> CC: john.fastabend@gmail.com
> CC: jakub@cloudflare.com
> CC: yoshfuji@linux-ipv6.org
> CC: dsahern@kernel.org
> CC: ast@kernel.org
> CC: daniel@iogearbox.net
> CC: andrii@kernel.org
> CC: kafai@fb.com
> CC: songliubraving@fb.com
> CC: yhs@fb.com
> CC: kpsingh@kernel.org
> CC: borisp@nvidia.com
> CC: cong.wang@bytedance.com
> CC: bpf@vger.kernel.org
> ---
>  include/net/inet_sock.h | 5 +++++
>  net/core/skmsg.c        | 5 +++++
>  net/ipv4/tcp_bpf.c      | 3 ---
>  net/tls/tls_main.c      | 2 ++
>  4 files changed, 12 insertions(+), 3 deletions(-)

Thanks Jakub this looks correct to me. I can't remember at the moment
or dig up in the email or git why it was originally moved into the
update logic.  Maybe Cong can comment seeing it was his original patch?
I seemed to have missed the error path on original review :(

From my side though,

Reviewed-by: John Fastabend <john.fastabend@gmail.com>