Re: [PATCH net 1/5] net/sched: act_vlan: reject short linear Ethernet headers after POP_ETH

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

Jamal Hadi Salim wrote:
> The issue:
> act_vlan POP_ETH can leave a linear area shorter than ETH_HLEN; a
> subsequent eth_type_trans() may then BUG in __skb_pull() when the frame
> is redirected to an Ethernet device.
> 
> The Fix:
> Drop the packet at the producer boundary when skb_headlen() is shorter
> than ETH_HLEN after skb_eth_pop().
> 
> Conditions to recreate the bug:
> - egress tc filter on lo: action vlan pop_eth.
> - AF_PACKET SOCK_RAW with PACKET_VNET_HDR, hdr_len=28, and a non-linear
>   eth / inner-eth frame.
> - The pop leaves skb_headlen(skb) < ETH_HLEN; a following
>   eth_type_trans() can BUG in __skb_pull().
> 
> Fixes: 19fbcb36a39e ("net/sched: act_vlan: Add {POP,PUSH}_ETH actions")
> Reported-by: Vega <vega@nebusec.ai>
> Link: https://lore.kernel.org/r/40fe2d7dfce13411bfbe9271c0f7c54fb7e88e3b.1782548651.git.bronzed_45_vested@icloud.com/
> Cc: stable@vger.kernel.org
> Suggested-by: Jakub Kicinski <kuba@kernel.org>
> Reviewed-by: Victor Nogueira <victor@mojatatu.com>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> ---
>  net/sched/act_vlan.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
> index 6a375fdc63dd..ddf03b08bfe8 100644
> --- a/net/sched/act_vlan.c
> +++ b/net/sched/act_vlan.c
> @@ -79,6 +79,8 @@ TC_INDIRECT_SCOPE int tcf_vlan_act(struct sk_buff *skb,
>  		err = skb_eth_pop(skb);
>  		if (err)
>  			goto drop;
> +		if (unlikely(skb_headlen(skb) < ETH_HLEN))
> +			goto drop;

Could it try to pull the header and only drop if the packet is too short?

>  		break;
>  	case TCA_VLAN_ACT_PUSH_ETH:
>  		err = skb_eth_push(skb, p->tcfv_push_dst, p->tcfv_push_src);
> -- 
> 2.43.0
>