netdev - Re: [PATCH v1 net] calipso: Fix null-ptr-deref in calipso_req

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: <20250617212334.1910048-1-kuni1840@gmail.com>
Date: Tue, 17 Jun 2025 14:22:36 -0700
From: Kuniyuki Iwashima <kuni1840@...il.com>
To: paul@...l-moore.com
Cc: davem@...emloft.net,
	dsahern@...nel.org,
	edumazet@...gle.com,
	horms@...nel.org,
	huw@...eweavers.com,
	john.cs.hey@...il.com,
	kuba@...nel.org,
	kuni1840@...il.com,
	kuniyu@...gle.com,
	linux-security-module@...r.kernel.org,
	netdev@...r.kernel.org,
	pabeni@...hat.com,
	syzkaller@...glegroups.com
Subject: Re: [PATCH v1 net] calipso: Fix null-ptr-deref in calipso_req_{set,del}attr().

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