Re: [PATCH v1 net] calipso: Fix null-ptr-deref in calipso_req_{set,del}attr().

[Kuniyuki Iwashima <kuni1840@gmail.com>]

From: Paul Moore <paul@paul-moore.com>
Date: Tue, 17 Jun 2025 17:04:18 -0400
> On Mon, Jun 16, 2025 at 1:26 PM Kuniyuki Iwashima <kuni1840@gmail.com> wrote:
> >
> > 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 */

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.