Willem de Bruijn
2021-Mar-08 16:01 UTC
[PATCH v2 1/2] net: check if protocol extracted by virtio_net_hdr_set_proto is correct
On Mon, Mar 8, 2021 at 5:32 AM Balazs Nemeth <bnemeth at redhat.com> wrote:> > For gso packets, virtio_net_hdr_set_proto sets the protocol (if it isn't > set) based on the type in the virtio net hdr, but the skb could contain > anything since it could come from packet_snd through a raw socket. If > there is a mismatch between what virtio_net_hdr_set_proto sets and > the actual protocol, then the skb could be handled incorrectly later > on. > > An example where this poses an issue is with the subsequent call to > skb_flow_dissect_flow_keys_basic which relies on skb->protocol being set > correctly. A specially crafted packet could fool > skb_flow_dissect_flow_keys_basic preventing EINVAL to be returned. > > Avoid blindly trusting the information provided by the virtio net header > by checking that the protocol in the packet actually matches the > protocol set by virtio_net_hdr_set_proto. Note that since the protocol > is only checked if skb->dev implements header_ops->parse_protocol, > packets from devices without the implementation are not checked at this > stage. > > Fixes: 9274124f023b ("net: stricter validation of untrusted gso packets") > Signed-off-by: Balazs Nemeth <bnemeth at redhat.com>Going forward, please mark your the patch as targeting the net tree using [PATCH net]> --- > include/linux/virtio_net.h | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h > index e8a924eeea3d..6c478eee0452 100644 > --- a/include/linux/virtio_net.h > +++ b/include/linux/virtio_net.h > @@ -79,8 +79,14 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, > if (gso_type && skb->network_header) { > struct flow_keys_basic keys; > > - if (!skb->protocol) > + if (!skb->protocol) { > + const struct ethhdr *eth = skb_eth_hdr(skb);eth is no longer used.> + __be16 etype = dev_parse_header_protocol(skb);nit: customary to call this protocol. etype, I guess short for EtherType, makes sense, but is not commonly used in the kernel.> + > virtio_net_hdr_set_proto(skb, hdr); > + if (etype && etype != skb->protocol) > + return -EINVAL; > + } > retry: > if (!skb_flow_dissect_flow_keys_basic(NULL, skb, &keys, > NULL, 0, 0, 0, > -- > 2.29.2 >