netdev - Re: [PATCH v1 net] smc: Fix various oops due to inet

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] [day] [month] [year] [list]

Message-ID: <CAAVpQUDrtvVadYGG1ZfL=usU11jbN-bZ=FQ4n5+1Ammry-w1yQ@mail.gmail.com>
Date: Tue, 15 Jul 2025 10:07:05 -0700
From: Kuniyuki Iwashima <kuniyu@...gle.com>
To: "D. Wythe" <alibuda@...ux.alibaba.com>
Cc: Alexandra Winter <wintera@...ux.ibm.com>, Dust Li <dust.li@...ux.alibaba.com>, 
	Sidraya Jayagond <sidraya@...ux.ibm.com>, Wenjia Zhang <wenjia@...ux.ibm.com>, 
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, 
	Mahanta Jambigi <mjambigi@...ux.ibm.com>, Tony Lu <tonylu@...ux.alibaba.com>, 
	Wen Gu <guwen@...ux.alibaba.com>, Simon Horman <horms@...nel.org>, 
	Kuniyuki Iwashima <kuni1840@...il.com>, netdev@...r.kernel.org, linux-rdma@...r.kernel.org, 
	linux-s390@...r.kernel.org, 
	syzbot+40bf00346c3fe40f90f2@...kaller.appspotmail.com, 
	syzbot+f22031fad6cbe52c70e7@...kaller.appspotmail.com, 
	syzbot+271fed3ed6f24600c364@...kaller.appspotmail.com
Subject: Re: [PATCH v1 net] smc: Fix various oops due to inet_sock type confusion.

On Tue, Jul 15, 2025 at 4:59 AM D. Wythe <alibuda@...ux.alibaba.com> wrote:
>
> On Mon, Jul 14, 2025 at 09:42:22AM +0200, Alexandra Winter wrote:
> >
> >
> > On 11.07.25 08:07, Kuniyuki Iwashima wrote:
> > > syzbot reported weird splats [0][1] in cipso_v4_sock_setattr() while
> > > freeing inet_sk(sk)->inet_opt.
> > >
> > > The address was freed multiple times even though it was read-only memory.
> > >
> > > cipso_v4_sock_setattr() did nothing wrong, and the root cause was type
> > > confusion.
> > >
> > > The cited commit made it possible to create smc_sock as an INET socket.
> > >
> > > The issue is that struct smc_sock does not have struct inet_sock as the
> > > first member but hijacks AF_INET and AF_INET6 sk_family, which confuses
> > > various places.
> > >
> > > In this case, inet_sock.inet_opt was actually smc_sock.clcsk_data_ready(),
> >
> > I would like to remind us of the discussions August 2024 around a patchset
> > called "net/smc: prevent NULL pointer dereference in txopt_get".
> > That discussion eventually ended up in the reduced (?)
> > commit 98d4435efcbf ("net/smc: prevent NULL pointer dereference in txopt_get")
> > without a union.
> >
> > I still think this union looks dangerous, but don't understand the code well enough to
> > propose an alternative.
> >
> > Maybe incorporate inet_sock in smc_sock? Like Paoplo suggested in
> > https://lore.kernel.org/lkml/20240815043714.38772-1-aha310510@gmail.com/T/#maf6ee926f782736cb6accd2ba162dea0a34e02f9
> >
> > He also asked for at least some explanatory comments in the union. Which would help me as well.
> >
>
> Just caught this suggestion... The primary risk with using a union is the
> potential for the sk member's offset within the inet_sock structure to
> change in the future, although this is highly improbable.

Right, and this only happens when we start using inet_sock in the union.


> But in any
> case, directly using inet_sock is certainly a safer approach.
>
> Uncertain if @Kuniyuki will still get to revise a version, If there's no further
> follow-up, I'll make the changes when I get a change.

I'll post a follow-up once net.git is merged to net-next.