Re: [PATCH NET] vrf: fix null pointer dereference in vrf_finish_output()

[Jakub Kicinski]

On Fri, 6 Aug 2021 15:53:00 +0300 Vasily Averin wrote:
> After 14ee70ca89e6 ("vrf: use skb_expand_head in vrf_finish_output")
> skb->dev  is accessed after skb free.
> Let's replace skb->dev by dev = skb_dst(skb)->dev:
> vrf_finish_output() is only called from vrf_output(),
> it set skb->dev to skb_dst(skb)->dev and calls POSTROUTING netfilter
> hooks, where output device should not be changed.
> 
> Fixes: 14ee70ca89e6 ("vrf: use skb_expand_head in vrf_finish_output")
> Reported-by: Julian Wiedmann <jwi@xxxxxxxxxxxxx>
> Signed-off-by: Vasily Averin <vvs@xxxxxxxxxxxxx>

Thanks for following up! I decided to pick a similar patch from Dan
Carpenter [1] because the chunk quoted below is not really necessary.

[1] https://lore.kernel.org/kernel-janitors/20210806150435.GB15586@kili/

> @@ -883,7 +883,7 @@ static int vrf_finish_output(struct net *net, struct sock *sk, struct sk_buff *s
>  	}
>  
>  	rcu_read_unlock_bh();
> -	vrf_tx_error(skb->dev, skb);
> +	vrf_tx_error(dev, skb);
>  	return -EINVAL;
>  }
>