netdev - Re: [PATCH v2 net-next 2/3] pfcp: Convert pfcp_net_exit() to ->exit

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: <20250424022531.93945-1-kuniyu@amazon.com>
Date: Wed, 23 Apr 2025 19:23:28 -0700
From: Kuniyuki Iwashima <kuniyu@...zon.com>
To: <kuba@...nel.org>
CC: <andrew+netdev@...n.ch>, <davem@...emloft.net>, <edumazet@...gle.com>,
	<horms@...nel.org>, <kuni1840@...il.com>, <kuniyu@...zon.com>,
	<netdev@...r.kernel.org>, <pabeni@...hat.com>
Subject: Re: [PATCH v2 net-next 2/3] pfcp: Convert pfcp_net_exit() to ->exit_rtnl().

From: Jakub Kicinski <kuba@...nel.org>
Date: Wed, 23 Apr 2025 15:52:33 -0700
> On Wed, 23 Apr 2025 07:12:55 -0700 Kuniyuki Iwashima wrote:
> > > >  	list_for_each_entry_safe(pfcp, pfcp_next, &pn->pfcp_dev_list, list)
> > > > -		pfcp_dellink(pfcp->dev, &list);
> > > > -
> > > > -	unregister_netdevice_many(&list);
> > > > -	rtnl_unlock();
> > > > +		pfcp_dellink(pfcp->dev, dev_to_kill);  
> > > 
> > > Kuniyuki, I got distracted by the fact the driver is broken but I think
> > > this isn't right.  
> > 
> > I guess it was broken recently ?  at least I didn't see null-deref
> > while testing ffc90e9ca61b ("pfcp: Destroy device along with udp
> > socket's netns dismantle.").
> 
> Not sure, nothing seems to have changed since?

It's been broken since the first commit of pfcp, but the bug seems
to be exposed recently by the commit below, which changed the per-cpu
variable section address from 0 to relative address.

  $ git bisect good
  9d7de2aa8b41407bc96d89a80dc1fd637d389d42 is the first bad commit
  commit 9d7de2aa8b41407bc96d89a80dc1fd637d389d42
  Author: Brian Gerst <brgerst@...il.com>
  Date:   Thu Jan 23 14:07:40 2025 -0500

      x86/percpu/64: Use relative percpu offsets

Looks like before this commit 0 was a valid per-cpu variable address
on x86, and that's why accessing per_cpu_ptr(NULL, cpu) was handled
(im)properly.

The fix is one-liner assigning pcpu_stat_type, but no one have used
pfcp for the recent 3 months and haven't noticed the wrong stats nor
used stats for a year.

Do we want to fix it or remove ? :)