netdev - Re: [PATCH net-next] udp_tunnel: properly deal with xfrm gro encap.

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: <fa4a449a-9af9-4106-924e-97e14e7fe7c0@redhat.com>
Date: Wed, 19 Mar 2025 23:15:00 +0100
From: Paolo Abeni <pabeni@...hat.com>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>, netdev@...r.kernel.org
Cc: "David S. Miller" <davem@...emloft.net>, David Ahern
 <dsahern@...nel.org>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Simon Horman <horms@...nel.org>,
 Willem de Bruijn <willemb@...gle.com>, steffen.klassert@...unet.com
Subject: Re: [PATCH net-next] udp_tunnel: properly deal with xfrm gro encap.

On 3/19/25 6:44 PM, Willem de Bruijn wrote:
> Paolo Abeni wrote:
>> Given syzkaller has found another splat with no reproducer on the other
>> UDP GRO change of mine [1] and we are almost at merge window time, I'm
>> considering reverting entirely such changes and re-submit later
>> (hopefully fixed). WDYT?
> 
> Your call. I suspect that we can forward fix this. But yes, that is
> always the riskier approach. And from a first quick look at the
> report, the right fix is not immediately glaringly obvious indeed.

One problem to me is that I have my hands significantly full, since the
revert looks like the faster way out it looks the more appealing
candidate to me.

WRT the other issue, I think the problem is in udp_tunnel_cleanup_gro();
this check:

        if (!up->tunnel_list.pprev)
                return;

at sk deletion time is performed outside any lock. The current CPU could
see the old list value (empty) even if another core previously added the
sk into the UDP tunnel GRO list, thus skipping the removal from such
list. Later list operation will do UaF while touching the same list.

Moving the check under the udp_tunnel_gro_lock spinlock should solve the
issue.

Thanks,

Paolo