[bug report] xfrm: Fix oops in __xfrm_state_delete()

[Dan Carpenter <error27@xxxxxxxxx>]

Hello Thomas Jarosch,

The patch c39f95aaf6d1: "xfrm: Fix oops in __xfrm_state_delete()"
from Nov 2, 2022, leads to the following Smatch static checker
warning:

	net/xfrm/xfrm_policy.c:2139 xfrm_policy_lookup_bytype()
	error: potential NULL/IS_ERR bug 'ret'

net/xfrm/xfrm_policy.c
    2076 static struct xfrm_policy *xfrm_policy_lookup_bytype(struct net *net, u8 type,
    2077                                                      const struct flowi *fl,
    2078                                                      u16 family, u8 dir,
    2079                                                      u32 if_id)
    2080 {
    2081         struct xfrm_pol_inexact_candidates cand;
    2082         const xfrm_address_t *daddr, *saddr;
    2083         struct xfrm_pol_inexact_bin *bin;
    2084         struct xfrm_policy *pol, *ret;
    2085         struct hlist_head *chain;
    2086         unsigned int sequence;
    2087         int err;
    2088 
    2089         daddr = xfrm_flowi_daddr(fl, family);
    2090         saddr = xfrm_flowi_saddr(fl, family);
    2091         if (unlikely(!daddr || !saddr))
    2092                 return NULL;
    2093 
    2094         rcu_read_lock();
    2095  retry:
    2096         do {
    2097                 sequence = read_seqcount_begin(&net->xfrm.xfrm_policy_hash_generation);
    2098                 chain = policy_hash_direct(net, daddr, saddr, family, dir);
    2099         } while (read_seqcount_retry(&net->xfrm.xfrm_policy_hash_generation, sequence));
    2100 
    2101         ret = NULL;

ret starts as NULL.

    2102         hlist_for_each_entry_rcu(pol, chain, bydst) {
    2103                 err = xfrm_policy_match(pol, fl, type, family, if_id);
    2104                 if (err) {
    2105                         if (err == -ESRCH)
    2106                                 continue;

I guess it could end as NULL if every err == -ESRCH.

    2107                         else {
    2108                                 ret = ERR_PTR(err);
    2109                                 goto fail;
    2110                         }
    2111                 } else {
    2112                         ret = pol;
    2113                         break;
    2114                 }
    2115         }
    2116         bin = xfrm_policy_inexact_lookup_rcu(net, type, family, dir, if_id);
    2117         if (!bin || !xfrm_policy_find_inexact_candidates(&cand, bin, saddr,
    2118                                                          daddr))
    2119                 goto skip_inexact;
    2120 
    2121         pol = xfrm_policy_eval_candidates(&cand, ret, fl, type,
    2122                                           family, if_id);
    2123         if (pol) {
    2124                 ret = pol;
    2125                 if (IS_ERR(pol))
    2126                         goto fail;
    2127         }
    2128 
    2129 skip_inexact:
    2130         if (read_seqcount_retry(&net->xfrm.xfrm_policy_hash_generation, sequence))
    2131                 goto retry;
    2132 
    2133         if (ret && !xfrm_pol_hold_rcu(ret))
                     ^^^
This code says it can be NULL.

    2134                 goto retry;
    2135 fail:
    2136         rcu_read_unlock();
    2137 
    2138         if (!IS_ERR(ret))
                             ^^^
So this should be if (!IS_ERR_OR_NULL(ret))

--> 2139                 printk("xfrm_policy_lookup_bytype: policy if_id %d, wanted if_id  %d\n", ret->if_id, if_id);
                                                                                                  ^^^^^
Unchecked dereference.

    2140 
    2141         return ret;
    2142 }

regards,
dan carpenter