Re: [PATCH net-next v2] af_unix: Fix undefined 'other' error

[Simon Horman <horms@kernel.org>]

On Sun, Feb 16, 2025 at 10:33:38PM +0300, Dan Carpenter wrote:
> I've added the linux-sparse@vger.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.