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: <20250217111515.GI1615191@kernel.org>
Date: Mon, 17 Feb 2025 11:15:15 +0000
From: Simon Horman <horms@...nel.org>
To: Dan Carpenter <dan.carpenter@...aro.org>
Cc: Purva Yeshi <purvayeshi550@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	skhan@...uxfoundation.org, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, Kuniyuki Iwashima <kuniyu@...zon.com>,
	linux-sparse@...r.kernel.org
Subject: Re: [PATCH net-next v2] af_unix: Fix undefined 'other' error

On Sun, Feb 16, 2025 at 10:33:38PM +0300, Dan Carpenter wrote:
> I've added the linux-sparse@...r.kernel.org mailing list to the CC.
> 
> On Sat, Feb 15, 2025 at 05:24:40PM +0000, Simon Horman wrote:
> > My understanding is that the two static analysis tools under discussion
> > are Smatch and Sparse, where AFAIK Smatch is a fork of Sparse.
> > 
> > Without this patch, when checking af_unix.c, both Smatch and Sparse report
> > (only):
> > 
> >  .../af_unix.c:1511:9: error: undefined identifier 'other'
> >  .../af_unix.c:1511:9: error: undefined identifier 'other'
> >  .../af_unix.c:1511:9: error: undefined identifier 'other'
> >  .../af_unix.c:1511:9: error: undefined identifier 'other'
> > 
> 
> Smatch isn't a fork of Sparse, it uses Sparse as a C front-end.

Sorry for my mistake there.

> This warning is really from Sparse, not Smatch.  The warning started
> when we changed the definition of unix_sk() in commit b064ba9c3cfa
> ("af_unix: preserve const qualifier in unix_sk()").
> 
> Smatch doesn't actually use these locking annotations at all.  Instead,
> Smatch has a giant table with all the locks listed.
> https://github.com/error27/smatch/blob/master/smatch_locking.c
> Smatch uses the cross function database for this as well if it's
> available.
> 
> Unfortunately, Smatch does not parse the unix_wait_for_peer() function
> correctly.  It sees that something is unlocked but it can't figure out
> what.  I believe the problem is that Smatch doesn't parse
> container_of_const().  Fixing that has been on my TODO list for a while.
> The caller used unix_state_lock() to take the lock and that has a
> unix_sk() in it as well.  So smatch doesn't see this lock at all that's
> why it doesn't print a warning.

So, hypothetically, Smatch could be enhanced and there wouldn't be any
locking warnings with this patch applied?

> 
> regards,
> dan carpenter
> 
> > Without this patch, when checking af_unix.c, both Smatch and Sparse report
> > (only):
> > 
> >  .../af_unix.c:1511:9: error: undefined identifier 'other'
> >  .../af_unix.c:1511:9: error: undefined identifier 'other'
> >  .../af_unix.c:1511:9: error: undefined identifier 'other'
> >  .../af_unix.c:1511:9: error: undefined identifier 'other'
> > 
> > And with either v1 or v2 of this patch applied Smatch reports nothing.
> > While Sparse reports:
> > 
> >  .../af_unix.c:234:13: warning: context imbalance in 'unix_table_double_lock' - wrong count at exit
> >  .../af_unix.c:253:28: warning: context imbalance in 'unix_table_double_unlock' - unexpected unlock
> >  .../af_unix.c:1386:13: warning: context imbalance in 'unix_state_double_lock' - wrong count at exit
> >  .../af_unix.c:1403:17: warning: context imbalance in 'unix_state_double_unlock' - unexpected unlock
> >  .../af_unix.c:2089:25: warning: context imbalance in 'unix_dgram_sendmsg' - unexpected unlock
> >  .../af_unix.c:3335:20: warning: context imbalance in 'unix_get_first' - wrong count at exit
> >  .../af_unix.c:3366:34: warning: context imbalance in 'unix_get_next' - unexpected unlock
> >  .../af_unix.c:3396:42: warning: context imbalance in 'unix_seq_stop' - unexpected unlock
> >  .../af_unix.c:3499:34: warning: context imbalance in 'bpf_iter_unix_hold_batch' - unexpected unlock
> > 
> > TBH, I'm unsure which is worse. Nor how to improve things.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ