From d2aa125d629080c4f3e31f23b7f612ef6b8492ac Mon Sep 17 00:00:00 2001 From: Maxim Mikityanskiy Date: Thu, 21 Feb 2019 12:39:57 +0000 Subject: net: Don't set transport offset to invalid value If the socket was created with socket(AF_PACKET, SOCK_RAW, 0), skb->protocol will be unset, __skb_flow_dissect() will fail, and skb_probe_transport_header() will fall back to the offset_hint, making the resulting skb_transport_offset incorrect. If, however, there is no transport header in the packet, transport_header shouldn't be set to an arbitrary value. Fix it by leaving the transport offset unset if it couldn't be found, to be explicit rather than to fill it with some wrong value. It changes the behavior, but if some code relied on the old behavior, it would be broken anyway, as the old one is incorrect. Signed-off-by: Maxim Mikityanskiy Signed-off-by: David S. Miller --- drivers/net/tap.c | 4 ++-- drivers/net/tun.c | 4 ++-- drivers/net/xen-netback/netback.c | 15 ++++++++++++--- include/linux/skbuff.h | 5 +---- include/linux/virtio_net.h | 2 +- net/packet/af_packet.c | 6 +++--- 6 files changed, 21 insertions(+), 15 deletions(-) diff --git a/drivers/net/tap.c b/drivers/net/tap.c index c0b52e48f0e6..2ea9b4976f4a 100644 --- a/drivers/net/tap.c +++ b/drivers/net/tap.c @@ -712,7 +712,7 @@ static ssize_t tap_get_user(struct tap_queue *q, void *msg_control, goto err_kfree; } - skb_probe_transport_header(skb, ETH_HLEN); + skb_probe_transport_header(skb); /* Move network header to the right position for VLAN tagged packets */ if ((skb->protocol == htons(ETH_P_8021Q) || @@ -1187,7 +1187,7 @@ static int tap_get_user_xdp(struct tap_queue *q, struct xdp_buff *xdp) tap = rcu_dereference(q->tap); if (tap) { skb->dev = tap->dev; - skb_probe_transport_header(skb, ETH_HLEN); + skb_probe_transport_header(skb); dev_queue_xmit(skb); } else { kfree_skb(skb); diff --git a/drivers/net/tun.c b/drivers/net/tun.c index fed298c0cb39..80bff1b4ec17 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -1929,7 +1929,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile, } skb_reset_network_header(skb); - skb_probe_transport_header(skb, 0); + skb_probe_transport_header(skb); if (skb_xdp) { struct bpf_prog *xdp_prog; @@ -2482,7 +2482,7 @@ build: skb->protocol = eth_type_trans(skb, tun->dev); skb_reset_network_header(skb); - skb_probe_transport_header(skb, 0); + skb_probe_transport_header(skb); if (skb_xdp) { err = do_xdp_generic(xdp_prog, skb); diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c index 80aae3a32c2a..c801a832851c 100644 --- a/drivers/net/xen-netback/netback.c +++ b/drivers/net/xen-netback/netback.c @@ -1169,15 +1169,24 @@ static int xenvif_tx_submit(struct xenvif_queue *queue) continue; } - skb_probe_transport_header(skb, 0); + skb_probe_transport_header(skb); /* If the packet is GSO then we will have just set up the * transport header offset in checksum_setup so it's now * straightforward to calculate gso_segs. */ if (skb_is_gso(skb)) { - int mss = skb_shinfo(skb)->gso_size; - int hdrlen = skb_transport_header(skb) - + int mss, hdrlen; + + /* GSO implies having the L4 header. */ + WARN_ON_ONCE(!skb_transport_header_was_set(skb)); + if (unlikely(!skb_transport_header_was_set(skb))) { + kfree_skb(skb); + continue; + } + + mss = skb_shinfo(skb)->gso_size; + hdrlen = skb_transport_header(skb) - skb_mac_header(skb) + tcp_hdrlen(skb); diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index 2069fb90a559..27beb549ffbe 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -2429,8 +2429,7 @@ static inline void skb_pop_mac_header(struct sk_buff *skb) skb->mac_header = skb->network_header; } -static inline void skb_probe_transport_header(struct sk_buff *skb, - const int offset_hint) +static inline void skb_probe_transport_header(struct sk_buff *skb) { struct flow_keys_basic keys; @@ -2439,8 +2438,6 @@ static inline void skb_probe_transport_header(struct sk_buff *skb, if (skb_flow_dissect_flow_keys_basic(skb, &keys, NULL, 0, 0, 0, 0)) skb_set_transport_header(skb, keys.control.thoff); - else if (offset_hint >= 0) - skb_set_transport_header(skb, offset_hint); } static inline void skb_mac_header_rebuild(struct sk_buff *skb) diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h index 71f2394abbf7..6728bf581e98 100644 --- a/include/linux/virtio_net.h +++ b/include/linux/virtio_net.h @@ -62,7 +62,7 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb, * probe and drop if does not match one of the above types. */ if (gso_type) { - skb_probe_transport_header(skb, -1); + skb_probe_transport_header(skb); if (!skb_transport_header_was_set(skb)) return -EINVAL; } diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index 1cd1d83a4be0..6afd6369d19e 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -1970,7 +1970,7 @@ retry: if (unlikely(extra_len == 4)) skb->no_fcs = 1; - skb_probe_transport_header(skb, 0); + skb_probe_transport_header(skb); dev_queue_xmit(skb); rcu_read_unlock(); @@ -2519,7 +2519,7 @@ static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb, len = ((to_write > len_max) ? len_max : to_write); } - skb_probe_transport_header(skb, 0); + skb_probe_transport_header(skb); return tp_len; } @@ -2925,7 +2925,7 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len) virtio_net_hdr_set_proto(skb, &vnet_hdr); } - skb_probe_transport_header(skb, reserve); + skb_probe_transport_header(skb); if (unlikely(extra_len == 4)) skb->no_fcs = 1; -- cgit v1.2.3 From e78b2915517e8fcadb1bc130ad6aeac7099e510c Mon Sep 17 00:00:00 2001 From: Maxim Mikityanskiy Date: Thu, 21 Feb 2019 12:39:58 +0000 Subject: net: Introduce parse_protocol header_ops callback Introduce a new optional header_ops callback called parse_protocol and a wrapper function dev_parse_header_protocol, similar to dev_parse_header. The new callback's purpose is to extract the protocol number from the L2 header, the format of which is known to the driver, but not to the upper layers of the stack. Signed-off-by: Maxim Mikityanskiy Signed-off-by: David S. Miller --- include/linux/netdevice.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index aab4d9f6613d..6997f62cb6a0 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -274,6 +274,7 @@ struct header_ops { const struct net_device *dev, const unsigned char *haddr); bool (*validate)(const char *ll_header, unsigned int len); + __be16 (*parse_protocol)(const struct sk_buff *skb); }; /* These flag bits are private to the generic network queueing @@ -2939,6 +2940,15 @@ static inline int dev_parse_header(const struct sk_buff *skb, return dev->header_ops->parse(skb, haddr); } +static inline __be16 dev_parse_header_protocol(const struct sk_buff *skb) +{ + const struct net_device *dev = skb->dev; + + if (!dev->header_ops || !dev->header_ops->parse_protocol) + return 0; + return dev->header_ops->parse_protocol(skb); +} + /* ll_header must have at least hard_header_len allocated */ static inline bool dev_validate_header(const struct net_device *dev, char *ll_header, int len) -- cgit v1.2.3 From ace53b2e2945c83850964070af158be01d564e67 Mon Sep 17 00:00:00 2001 From: Maxim Mikityanskiy Date: Thu, 21 Feb 2019 12:39:59 +0000 Subject: net/ethernet: Add parse_protocol header_ops support The previous commit introduced parse_protocol callback which should extract the protocol number from the L2 header. Make all Ethernet devices support it. Signed-off-by: Maxim Mikityanskiy Signed-off-by: David S. Miller --- include/linux/etherdevice.h | 1 + net/ethernet/eth.c | 13 +++++++++++++ 2 files changed, 14 insertions(+) diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h index 2c0af7b00715..e2f3b21cd72a 100644 --- a/include/linux/etherdevice.h +++ b/include/linux/etherdevice.h @@ -44,6 +44,7 @@ int eth_header_cache(const struct neighbour *neigh, struct hh_cache *hh, __be16 type); void eth_header_cache_update(struct hh_cache *hh, const struct net_device *dev, const unsigned char *haddr); +__be16 eth_header_parse_protocol(const struct sk_buff *skb); int eth_prepare_mac_addr_change(struct net_device *dev, void *p); void eth_commit_mac_addr_change(struct net_device *dev, void *p); int eth_mac_addr(struct net_device *dev, void *p); diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c index 4c520110b04f..f7a3d7a171c7 100644 --- a/net/ethernet/eth.c +++ b/net/ethernet/eth.c @@ -264,6 +264,18 @@ void eth_header_cache_update(struct hh_cache *hh, } EXPORT_SYMBOL(eth_header_cache_update); +/** + * eth_header_parser_protocol - extract protocol from L2 header + * @skb: packet to extract protocol from + */ +__be16 eth_header_parse_protocol(const struct sk_buff *skb) +{ + const struct ethhdr *eth = eth_hdr(skb); + + return eth->h_proto; +} +EXPORT_SYMBOL(eth_header_parse_protocol); + /** * eth_prepare_mac_addr_change - prepare for mac change * @dev: network device @@ -346,6 +358,7 @@ const struct header_ops eth_header_ops ____cacheline_aligned = { .parse = eth_header_parse, .cache = eth_header_cache, .cache_update = eth_header_cache_update, + .parse_protocol = eth_header_parse_protocol, }; /** -- cgit v1.2.3 From 75c65772c3d18447d62d3aca5f91b06c16cc25e4 Mon Sep 17 00:00:00 2001 From: Maxim Mikityanskiy Date: Thu, 21 Feb 2019 12:40:01 +0000 Subject: net/packet: Ask driver for protocol if not provided by user If a socket was created with socket(AF_PACKET, SOCK_RAW, 0), the protocol number is unavailable. Try to ask the driver to extract it from the L2 header in order for skb_try_probe_transport_header to succeed. Signed-off-by: Maxim Mikityanskiy Signed-off-by: David S. Miller --- net/packet/af_packet.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index 6afd6369d19e..cac4c1a3f807 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -1850,6 +1850,15 @@ oom: return 0; } +static void packet_parse_headers(struct sk_buff *skb, struct socket *sock) +{ + if (!skb->protocol && sock->type == SOCK_RAW) { + skb_reset_mac_header(skb); + skb->protocol = dev_parse_header_protocol(skb); + } + + skb_probe_transport_header(skb); +} /* * Output a raw packet to a device layer. This bypasses all the other @@ -1970,7 +1979,7 @@ retry: if (unlikely(extra_len == 4)) skb->no_fcs = 1; - skb_probe_transport_header(skb); + packet_parse_headers(skb, sock); dev_queue_xmit(skb); rcu_read_unlock(); @@ -2519,7 +2528,7 @@ static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb, len = ((to_write > len_max) ? len_max : to_write); } - skb_probe_transport_header(skb); + packet_parse_headers(skb, sock); return tp_len; } @@ -2925,7 +2934,7 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len) virtio_net_hdr_set_proto(skb, &vnet_hdr); } - skb_probe_transport_header(skb); + packet_parse_headers(skb, sock); if (unlikely(extra_len == 4)) skb->no_fcs = 1; -- cgit v1.2.3 From 6437866e493fa5aa5be03dfb94e1d686a7fc984b Mon Sep 17 00:00:00 2001 From: Maxim Mikityanskiy Date: Thu, 21 Feb 2019 12:40:02 +0000 Subject: net/packet: Remove redundant skb->protocol set c72219b75f introduced tpacket_set_protocol that parses the Ethernet L2 header and sets skb->protocol if it's unset. It is no longer needed since the introduction of packet_parse_headers. In case of SOCK_RAW and unset skb->protocol, packet_parse_headers asks the driver to tell the protocol number, and it's implemented for all Ethernet devices. As the old function supported only Ethernet, no functionality is lost. Signed-off-by: Maxim Mikityanskiy Signed-off-by: David S. Miller --- net/packet/af_packet.c | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index cac4c1a3f807..8376bc1c1508 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -2413,15 +2413,6 @@ static void tpacket_destruct_skb(struct sk_buff *skb) sock_wfree(skb); } -static void tpacket_set_protocol(const struct net_device *dev, - struct sk_buff *skb) -{ - if (dev->type == ARPHRD_ETHER) { - skb_reset_mac_header(skb); - skb->protocol = eth_hdr(skb)->h_proto; - } -} - static int __packet_snd_vnet_parse(struct virtio_net_hdr *vnet_hdr, size_t len) { if ((vnet_hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) && @@ -2492,8 +2483,6 @@ static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb, return err; if (!dev_validate_header(dev, skb->data, hdrlen)) return -EINVAL; - if (!skb->protocol) - tpacket_set_protocol(dev, skb); data += hdrlen; to_write -= hdrlen; -- cgit v1.2.3 From 3517dfe6f23608951044cbfc833fa108603441fa Mon Sep 17 00:00:00 2001 From: Maxim Mikityanskiy Date: Thu, 21 Feb 2019 12:40:04 +0000 Subject: net/mlx5e: Remove the wrong assumption about transport offset skb_transport_offset() == 0 is not a special value. The only special value is when skb->transport_header is ~0U, and it's checked by skb_transport_header_was_set(). Signed-off-by: Maxim Mikityanskiy Reviewed-by: Saeed Mahameed Signed-off-by: David S. Miller --- drivers/net/ethernet/mellanox/mlx5/core/en_tx.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c index c1334a8ac8f3..6ca834702306 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c @@ -172,15 +172,8 @@ static inline u16 mlx5e_calc_min_inline(enum mlx5_inline_modes mode, hlen += VLAN_HLEN; break; case MLX5_INLINE_MODE_IP: - /* When transport header is set to zero, it means no transport - * header. When transport header is set to 0xff's, it means - * transport header wasn't set. - */ - if (skb_transport_offset(skb)) { - hlen = mlx5e_skb_l3_header_offset(skb); - break; - } - /* fall through */ + hlen = mlx5e_skb_l3_header_offset(skb); + break; case MLX5_INLINE_MODE_L2: default: hlen = mlx5e_skb_l2_header_offset(skb); -- cgit v1.2.3 From 41f5f63cd17530d9eddaf99a71f5d069afd787d8 Mon Sep 17 00:00:00 2001 From: Maxim Mikityanskiy Date: Thu, 21 Feb 2019 12:40:05 +0000 Subject: net/mlx5e: Trust kernel regarding transport offset After AF_PACKET is fixed to calculate the transport header offset correctly, trust the value set by the kernel. If the offset wasn't set, it means there is no transport header in the packet. Signed-off-by: Maxim Mikityanskiy Reviewed-by: Saeed Mahameed Signed-off-by: David S. Miller --- drivers/net/ethernet/mellanox/mlx5/core/en_tx.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c index 6ca834702306..e7aae45a01f8 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c @@ -148,12 +148,8 @@ static inline int mlx5e_skb_l2_header_offset(struct sk_buff *skb) static inline int mlx5e_skb_l3_header_offset(struct sk_buff *skb) { - struct flow_keys keys; - if (skb_transport_header_was_set(skb)) return skb_transport_offset(skb); - else if (skb_flow_dissect_flow_keys(skb, &keys, 0)) - return keys.control.thoff; else return mlx5e_skb_l2_header_offset(skb); } -- cgit v1.2.3