On 18/05/2020 10:01, Boris Sukholitko wrote:
> __netif_receive_skb_core may change the skb pointer passed into it (e.g.
> in rx_handler). The original skb may be freed as a result of this
> operation.
>
> The callers of __netif_receive_skb_core may further process original skb
> by using pt_prev pointer returned by __netif_receive_skb_core thus
> leading to unpleasant effects.
>
> The solution is to pass skb by reference into __netif_receive_skb_core.
>
> Signed-off-by: Boris Sukholitko <boris.sukholi...@broadcom.com>
I think this patch is correct, but there's a couple of things I'd like
to see before adding my Ack.
Firstly, please add a Fixes: tag; I expect the relevant commit will be
88eb1944e18c ("net: core: propagate SKB lists through packet_type lookup")
but I'm not 100% sure so do check that yourself.
Secondly:
> @@ -5174,6 +5177,7 @@ static int __netif_receive_skb_core(struct sk_buff
> *skb, bool pfmemalloc,
> }
>
> out:
> + *pskb = skb;
> return ret;
> }
Could we have some sort of WARN_ONs (maybe under #ifdef DEBUG) to check
that we never have a NULL skb with a non-NULL pt_prev? Or at least a
comment at the top of the function stating this part of its contract
with callers? I've gone through and convinced myself that it never
happens currently, but that depends on some fairly subtle details.
-ed
