On Wed, Jan 31, 2024 at 08:38:51PM +0100, Antony Antony wrote:

HI Dan,

Thanks for reporting the warning.

On Tue, Jan 30, 2024 at 01:36:28PM +0300, Dan Carpenter wrote:

quoted

Hello Antony Antony,

The patch 63b21caba17e: "xfrm: introduce forwarding of ICMP Error
messages" from Jan 19, 2024 (linux-next), leads to the following
Smatch static checker warning:

	net/xfrm/xfrm_policy.c:3708 __xfrm_policy_check()
	error: testing array offset 'dir' after use.

quoted

net/xfrm/xfrm_policy.c
  3689  
  3690          pol = NULL;
  3691          sk = sk_to_full_sk(sk);
  3692          if (sk && sk->sk_policy[dir]) {
                            ^^^^^^^^^^^^^^^^
If dir is XFRM_POLICY_FWD (2) then it is one element beyond the end of
the ->sk_policy[] array.

Yes, that's correct. However, for this patch, it's necessary that sk != NULL 
at the same time. As far as I know, there isn't any code that would call dir 
= XFRM_POLICY_FWD with sk != NULL. What am I missing? Did Smatch give any 
hints for such a code path?

I wondered if that might be the case.  The truth is that this sort of
dependency is too compicated for any static analysis tools that
currently exist.  Smatch tries to track the relationship between
"dir" and "sk" as they are passed in, but it will look the relationship
information when we re-assign sk.  "sk = sk_to_full_sk(sk);".

So what we do in this case, is we just ignore the warning and if anyone
has questions about it they will look up this conversation on
lore.kernel.org to find the explanation.

No need to worry about trying to silence the checker...

regards,
dan carpenter