[<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