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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250617223601.14060-1-kuni1840@gmail.com>
Date: Tue, 17 Jun 2025 15:35:48 -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 18:08:15 -0400
> On Tue, Jun 17, 2025 at 5:23 PM Kuniyuki Iwashima <kuni1840@...il.com> wrote:
> > 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 */
> 
> 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.

Ok, will post v2 with comments and your tag.

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

Thanks!