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

[Dan Carpenter]

I've added the linux-sparse@xxxxxxxxxxxxxxx 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.
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.

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.