netdev - Re: [PATCH v6.1 05/27] af_unix: Replace BUG_ON() with WARN_ON

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: <20250604184528.141251-1-kuni1840@gmail.com>
Date: Wed,  4 Jun 2025 11:45:17 -0700
From: Kuniyuki Iwashima <kuni1840@...il.com>
To: lee@...nel.org
Cc: Rao.Shoaib@...cle.com,
	aleksandr.mikhalitsyn@...onical.com,
	axboe@...nel.dk,
	brauner@...nel.org,
	davem@...emloft.net,
	david.laight.linux@...il.com,
	edumazet@...gle.com,
	horms@...nel.org,
	kuba@...nel.org,
	kuniyu@...zon.com,
	linux-kernel@...r.kernel.org,
	mhal@...x.co,
	netdev@...r.kernel.org,
	pabeni@...hat.com,
	sashal@...nel.org,
	stable@...r.kernel.org
Subject: Re: [PATCH v6.1 05/27] af_unix: Replace BUG_ON() with WARN_ON_ONCE().

From: Lee Jones <lee@...nel.org>
Date: Wed, 4 Jun 2025 14:43:47 +0100
> On Fri, 23 May 2025, David Laight wrote:
> 
> > On Wed, 21 May 2025 16:27:04 +0100
> > Lee Jones <lee@...nel.org> wrote:
> > 
> > > From: Kuniyuki Iwashima <kuniyu@...zon.com>
> > > 
> > > [ Upstream commit d0f6dc26346863e1f4a23117f5468614e54df064 ]
> > > 
> > > This is a prep patch for the last patch in this series so that
> > > checkpatch will not warn about BUG_ON().
> > 
> > Does any of this actually make any sense?
> > Either the BUG_ON() should be just deleted because it can't happen
> > (or doesn't matter) or there should be an error path.
> > Blindly replacing with WARN_ON_ONCE() can't be right.
> > 
> > The last change (repeated here)
> > >  	if (u) {
> > > -		BUG_ON(!u->inflight);
> > > -		BUG_ON(list_empty(&u->link));
> > > +		WARN_ON_ONCE(!u->inflight);
> > > +		WARN_ON_ONCE(list_empty(&u->link));
> > >  
> > >  		u->inflight--;
> > >  		if (!u->inflight)
> > is clearly just plain wrong.
> > If 'inflight' is zero then 'decrementing' it to ~0 is just going
> > to 'crash and burn' very badly not much later on.
> 
> All of this gets removed in patch 20, so I fear the point is moot.

Right, and u->inflight never gets 0 before the decrementing in the
first place.