Re: [PATCH net-next v14 0/9] tls: Add TLS 1.3 hardware offload support

[Nils Juenemann <nils.juenemann@gmail.com>]

Hi Rishikesh, all,

Another NULL deref we hit while testing v14, on the RX resync path. Looks
pre-existing in mlx5e rather than introduced by v14. TLS 1.3 RX HW-kTLS,
mlx5/ConnectX; seen once in ~3 weeks of load:

    BUG: kernel NULL pointer dereference, address: 0000000000000030
    RIP: mlx5e_ktls_handle_rx_skb+0x10a [mlx5_core]
    Call Trace:
     <IRQ>
     mlx5e_build_rx_skb
     mlx5e_handle_rx_cqe_mpwrq
     mlx5e_poll_rx_cq
     mlx5e_napi_poll

It faults in the CQE_TLS_OFFLOAD_RESYNC branch, in resync_update_sn():

    resync_async = tls_offload_ctx_rx(tls_get_ctx(sk))->resync_async;

The NULL is tls_get_ctx(sk) itself: RAX=0 and the faulting instruction is
the priv_ctx_rx load off struct tls_context. CR2=0x30 matches
offsetof(struct tls_context, priv_ctx_rx), and [sk+0x4f8] is
icsk_ulp_data in the matching vmlinux.

So the socket returned by inet_lookup_established() no longer has a TLS
ULP context. resync_update_sn() reads tls_get_ctx(sk) twice: once in
resync_queue_get_psv() just above, which also dereferences it, and again
here, where it is NULL. This looks like icsk_ulp_data being cleared
between the two reads, i.e. RX resync racing TLS ULP teardown. In any
case, the second read is dereferenced without a NULL check.

I can send the full decoded oops or kdump specifics if that helps
narrow it down.

Thanks,
Nils Juenemann

PS: aside from this RX issue the series has been solid here. TLS 1.3
HW-kTLS TX has been running stably above 100 Gbit/s egress (~99.5% of
eligible TX traffic HW-offloaded) on AMD EPYC (Zen2) from a Go server
using a small in-house kTLS library and sendfile(). On this workload the
main win is not just AES offload, but avoiding ciphertext writes back
into DRAM: our measured DRAM-bytes/egress-bytes amplification drops from
~5.4x to ~2.2x. Thanks for the work on this.