On Tue, Jun 17, 2025 at 5:23 PM Kuniyuki Iwashima [off-list ref] wrote:

From: Paul Moore <paul@paul-moore.com>
Date: Tue, 17 Jun 2025 17:04:18 -0400

quoted

On Mon, Jun 16, 2025 at 1:26 PM Kuniyuki Iwashima [off-list ref] wrote:

quoted

From: Kuniyuki Iwashima <kuniyu@google.com>

syzkaller reported a null-ptr-deref in sock_omalloc() while allocating
a CALIPSO option.  [0]

The NULL is of struct sock, which was fetched by sk_to_full_sk() in
calipso_req_setattr().

Since commit a1a5344ddbe8 ("tcp: avoid two atomic ops for syncookies"),
reqsk->rsk_listener could be NULL when SYN Cookie is returned to its
client, as hinted by the leading SYN Cookie log.

Here are 3 options to fix the bug:

  1) Return 0 in calipso_req_setattr()
  2) Return an error in calipso_req_setattr()
  3) Alaways set rsk_listener

1) is no go as it bypasses LSM, but 2) effectively disables SYN Cookie
for CALIPSO.  3) is also no go as there have been many efforts to reduce
atomic ops and make TCP robust against DDoS.  See also commit 3b24d854cb35
("tcp/dccp: do not touch listener sk_refcnt under synflood").

As of the blamed commit, SYN Cookie already did not need refcounting,
and no one has stumbled on the bug for 9 years, so no CALIPSO user will
care about SYN Cookie.

Let's return an error in calipso_req_setattr() and calipso_req_delattr()
in the SYN Cookie case.

I think that's reasonable, but I think it would be nice to have a
quick comment right before the '!sk' checks to help people who may hit
the CALIPSO/SYN-cookie issue in the future.  Maybe "/*
tcp_syncookies=2 can result in sk == NULL */" ?

tcp_syncookies=1 enables SYN cookie and =2 forces it for every request.
I just used =2 to reproduce the issue without SYN flooding, so it would
be /* sk is NULL for SYN+ACK w/ SYN Cookie */

Sure, that sounds good.

But I think no one will hit it (at least so for 9 years) and wonder why
because SYN could be dropped randomly under such a event.

Yes, you are probably correct, but that doesn't mean a brief comment
as described above isn't a good idea.  If you add the comment and
you've got my ACK.

Acked-by: Paul Moore <paul@paul-moore.com>

-- 
paul-moore.com