Re: [PATCH net v2 3/5] net: use eth_type_trans_careful() in __dev_forward_skb2()

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

Eric Dumazet wrote:
> A tc egress BPF program can push a tiny link-layer header with
> bpf_skb_change_head() on an skb whose head is almost empty, then
> redirect it to an Ethernet device ingress. __bpf_redirect_common()
> only checks mac_header < network_header, and __dev_forward_skb2()
> crashes in eth_type_trans():
> 
>   tc filter add dev lo egress pref 1 matchall action vlan pop_eth continue
>   tc filter add dev lo egress pref 2 bpf da obj prog.o
>   # prog: bpf_skb_change_head(skb, 1, 0);
>   #       return bpf_redirect(veth1_ifindex, BPF_F_INGRESS);
>   # AF_PACKET SOCK_RAW on lo: eth / eth / 8192 bytes of payload
> 
>   kernel BUG at include/linux/skbuff.h:2839!
>   RIP: 0010:eth_type_trans (include/linux/etherdevice.h:635 net/ethernet/eth.c:163)
>   Call Trace:
>    __dev_forward_skb2 (net/core/dev.c:2507)
>    dev_forward_skb_nomtu (net/core/dev.c:2546)
>    __dev_queue_xmit (net/core/dev.c:4738 net/core/dev.c:4998)
>    packet_sendmsg (net/packet/af_packet.c:3157 net/packet/af_packet.c:3189)
> 
> Use eth_type_trans_careful(), and drop the packet if the Ethernet
> header can not be pulled. This also covers other dev_forward_skb()
> users (veth, macvlan, ipvlan, netdevsim, l2tp_eth).
> 
> Update the empty_skb BPF selftest: at lwt_xmit, BPF_PROG_TEST_RUN
> strips the Ethernet header, so the "veth ETH_HLEN+1" packet is a 1-byte
> skb. Redirecting it to veth ingress used to read the Ethernet header
> past skb->tail and deliver a 1-byte frame; it is now dropped with
> NET_RX_DROP, as veth_xmit() already does at egress.
> 
> Fixes: 3a0af8fd61f9 ("bpf: BPF for lightweight tunnel infrastructure")
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>
> Reviewed-by: Jamal Hadi Salim <jhs@mojatatu.com>
> Tested-by: Jamal Hadi Salim <jhs@mojatatu.com>

Reviewed-by: Willem de Bruijn <willemb@google.com>