the following script: # tc qdisc add dev eth0 clsact # tc filter add dev eth0 egress protocol ip matchall \ > action mpls push protocol mpls_uc label 0x355aa bos 1 causes corruption of all IP packets transmitted by eth0. On TC egress, we can't rely on the value of skb->mac_len, because it's 0 and a MPLS 'push' operation will result in an overwrite of the first 4 octets in the packet L2 header (e.g. the Destination Address if eth0 is an Ethernet); the same error pattern is present also in the MPLS 'pop' operation. Fix this error in act_mpls data plane, computing 'mac_len' as the difference between the network header and the mac header (when not at TC ingress), and use it in MPLS 'push'/'pop' core functions. v2: unbreak 'make htmldocs' because of missing documentation of 'mac_len' in skb_mpls_pop(), reported by kbuild test robot
Unified · Split
| @@ -3510,8 +3510,9 @@ int skb_ensure_writable(struct sk_buff *skb, int write_len); · 3509 unmodified lines | ||
| 3510 | 3510 | int __skb_vlan_pop(struct sk_buff *skb, u16 *vlan_tci); |
| 3511 | 3511 | int skb_vlan_pop(struct sk_buff *skb); |
| 3512 | 3512 | int skb_vlan_push(struct sk_buff *skb, __be16 vlan_proto, u16 vlan_tci); |
| 3513 | −int skb_mpls_push(struct sk_buff *skb, __be32 mpls_lse, __be16 mpls_proto); | |
| 3514 | −int skb_mpls_pop(struct sk_buff *skb, __be16 next_proto); | |
| 3513 | +int skb_mpls_push(struct sk_buff *skb, __be32 mpls_lse, __be16 mpls_proto, | |
| 3514 | + int mac_len); | |
| 3515 | +int skb_mpls_pop(struct sk_buff *skb, __be16 next_proto, int mac_len); | |
| 3515 | 3516 | int skb_mpls_update_lse(struct sk_buff *skb, __be32 mpls_lse); |
| 3516 | 3517 | int skb_mpls_dec_ttl(struct sk_buff *skb); |
| 3517 | 3518 | struct sk_buff *pskb_extract(struct sk_buff *skb, int off, int to_copy, |
| @@ -5477,12 +5477,14 @@ static void skb_mod_eth_type(struct sk_buff *skb, struct ethhdr *hdr, · 5476 unmodified lines | ||
| 5477 | 5477 | * @skb: buffer |
| 5478 | 5478 | * @mpls_lse: MPLS label stack entry to push |
| 5479 | 5479 | * @mpls_proto: ethertype of the new MPLS header (expects 0x8847 or 0x8848) |
| 5480 | + * @mac_len: length of the MAC header | |
| 5480 | 5481 | * |
| 5481 | 5482 | * Expects skb->data at mac header. |
| 5482 | 5483 | * |
| 5483 | 5484 | * Returns 0 on success, -errno otherwise. |
| 5484 | 5485 | */ |
| 5485 | −int skb_mpls_push(struct sk_buff *skb, __be32 mpls_lse, __be16 mpls_proto) | |
| 5486 | +int skb_mpls_push(struct sk_buff *skb, __be32 mpls_lse, __be16 mpls_proto, | |
| 5487 | + int mac_len) | |
| 5486 | 5488 | { |
| 5487 | 5489 | struct mpls_shim_hdr *lse; |
| 5488 | 5490 | int err; |
| @@ -5499,15 +5501,15 @@ int skb_mpls_push(struct sk_buff *skb, __be32 mpls_lse, __be16 mpls_proto) · 10 unmodified lines | ||
| 5499 | 5501 | return err; |
| 5500 | 5502 | |
| 5501 | 5503 | if (!skb->inner_protocol) { |
| 5502 | − skb_set_inner_network_header(skb, skb->mac_len); | |
| 5504 | + skb_set_inner_network_header(skb, mac_len); | |
| 5503 | 5505 | skb_set_inner_protocol(skb, skb->protocol); |
| 5504 | 5506 | } |
| 5505 | 5507 | |
| 5506 | 5508 | skb_push(skb, MPLS_HLEN); |
| 5507 | 5509 | memmove(skb_mac_header(skb) - MPLS_HLEN, skb_mac_header(skb), |
| 5508 | − skb->mac_len); | |
| 5510 | + mac_len); | |
| 5509 | 5511 | skb_reset_mac_header(skb); |
| 5510 | − skb_set_network_header(skb, skb->mac_len); | |
| 5512 | + skb_set_network_header(skb, mac_len); | |
| 5511 | 5513 | |
| 5512 | 5514 | lse = mpls_hdr(skb); |
| 5513 | 5515 | lse->label_stack_entry = mpls_lse; |
| @@ -5526,29 +5528,30 @@ EXPORT_SYMBOL_GPL(skb_mpls_push); · 12 unmodified lines | ||
| 5526 | 5528 | * |
| 5527 | 5529 | * @skb: buffer |
| 5528 | 5530 | * @next_proto: ethertype of header after popped MPLS header |
| 5531 | + * @mac_len: length of the MAC header | |
| 5529 | 5532 | * |
| 5530 | 5533 | * Expects skb->data at mac header. |
| 5531 | 5534 | * |
| 5532 | 5535 | * Returns 0 on success, -errno otherwise. |
| 5533 | 5536 | */ |
| 5534 | −int skb_mpls_pop(struct sk_buff *skb, __be16 next_proto) | |
| 5537 | +int skb_mpls_pop(struct sk_buff *skb, __be16 next_proto, int mac_len) | |
| 5535 | 5538 | { |
| 5536 | 5539 | int err; |
| 5537 | 5540 | |
| 5538 | 5541 | if (unlikely(!eth_p_mpls(skb->protocol))) |
| 5539 | 5542 | return 0; |
| 5540 | 5543 | |
| 5541 | − err = skb_ensure_writable(skb, skb->mac_len + MPLS_HLEN); | |
| 5544 | + err = skb_ensure_writable(skb, mac_len + MPLS_HLEN); | |
| 5542 | 5545 | if (unlikely(err)) |
| 5543 | 5546 | return err; |
| 5544 | 5547 | |
| 5545 | 5548 | skb_postpull_rcsum(skb, mpls_hdr(skb), MPLS_HLEN); |
| 5546 | 5549 | memmove(skb_mac_header(skb) + MPLS_HLEN, skb_mac_header(skb), |
| 5547 | − skb->mac_len); | |
| 5550 | + mac_len); | |
| 5548 | 5551 | |
| 5549 | 5552 | __skb_pull(skb, MPLS_HLEN); |
| 5550 | 5553 | skb_reset_mac_header(skb); |
| 5551 | − skb_set_network_header(skb, skb->mac_len); | |
| 5554 | + skb_set_network_header(skb, mac_len); | |
| 5552 | 5555 | |
| 5553 | 5556 | if (skb->dev && skb->dev->type == ARPHRD_ETHER) { |
| 5554 | 5557 | struct ethhdr *hdr; |
| @@ -165,7 +165,8 @@ static int push_mpls(struct sk_buff *skb, struct sw_flow_key *key, · 164 unmodified lines | ||
| 165 | 165 | { |
| 166 | 166 | int err; |
| 167 | 167 | |
| 168 | − err = skb_mpls_push(skb, mpls->mpls_lse, mpls->mpls_ethertype); | |
| 168 | + err = skb_mpls_push(skb, mpls->mpls_lse, mpls->mpls_ethertype, | |
| 169 | + skb->mac_len); | |
| 169 | 170 | if (err) |
| 170 | 171 | return err; |
| 171 | 172 | |
| @@ -178,7 +179,7 @@ static int pop_mpls(struct sk_buff *skb, struct sw_flow_key *key, · 6 unmodified lines | ||
| 178 | 179 | { |
| 179 | 180 | int err; |
| 180 | 181 | |
| 181 | − err = skb_mpls_pop(skb, ethertype); | |
| 182 | + err = skb_mpls_pop(skb, ethertype, skb->mac_len); | |
| 182 | 183 | if (err) |
| 183 | 184 | return err; |
| 184 | 185 | |
| @@ -55,7 +55,7 @@ static int tcf_mpls_act(struct sk_buff *skb, const struct tc_action *a, · 54 unmodified lines | ||
| 55 | 55 | struct tcf_mpls *m = to_mpls(a); |
| 56 | 56 | struct tcf_mpls_params *p; |
| 57 | 57 | __be32 new_lse; |
| 58 | − int ret; | |
| 58 | + int ret, mac_len; | |
| 59 | 59 | |
| 60 | 60 | tcf_lastuse_update(&m->tcf_tm); |
| 61 | 61 | bstats_cpu_update(this_cpu_ptr(m->common.cpu_bstats), skb); |
| @@ -63,8 +63,12 @@ static int tcf_mpls_act(struct sk_buff *skb, const struct tc_action *a, · 1 unmodified line | ||
| 63 | 63 | /* Ensure 'data' points at mac_header prior calling mpls manipulating |
| 64 | 64 | * functions. |
| 65 | 65 | */ |
| 66 | − if (skb_at_tc_ingress(skb)) | |
| 66 | + if (skb_at_tc_ingress(skb)) { | |
| 67 | 67 | skb_push_rcsum(skb, skb->mac_len); |
| 68 | + mac_len = skb->mac_len; | |
| 69 | + } else { | |
| 70 | + mac_len = skb_network_header(skb) - skb_mac_header(skb); | |
| 71 | + } | |
| 68 | 72 | |
| 69 | 73 | ret = READ_ONCE(m->tcf_action); |
| 70 | 74 | |
| @@ -72,12 +76,12 @@ static int tcf_mpls_act(struct sk_buff *skb, const struct tc_action *a, · 1 unmodified line | ||
| 72 | 76 | |
| 73 | 77 | switch (p->tcfm_action) { |
| 74 | 78 | case TCA_MPLS_ACT_POP: |
| 75 | − if (skb_mpls_pop(skb, p->tcfm_proto)) | |
| 79 | + if (skb_mpls_pop(skb, p->tcfm_proto, mac_len)) | |
| 76 | 80 | goto drop; |
| 77 | 81 | break; |
| 78 | 82 | case TCA_MPLS_ACT_PUSH: |
| 79 | 83 | new_lse = tcf_mpls_get_lse(NULL, p, !eth_p_mpls(skb->protocol)); |
| 80 | − if (skb_mpls_push(skb, new_lse, p->tcfm_proto)) | |
| 84 | + if (skb_mpls_push(skb, new_lse, p->tcfm_proto, mac_len)) | |
| 81 | 85 | goto drop; |
| 82 | 86 | break; |
| 83 | 87 | case TCA_MPLS_ACT_MODIFY: |