tcf_skbmod_act() calls skb_ensure_writable(skb, max_edit_len) to ensure the modified packet header is writable before rewriting Ethernet addresses or setting ECN bits. However, skb_ensure_writable() only pulls and COWs memory starting from skb->data onwards. At ingress or on forwarded packets, the Ethernet header (or network header) may reside in the headroom at a negative offset (skb_mac_offset(skb) < 0). Because skb_cow() is omitted for negative offsets, writes via ether_addr_copy() or INET_ECN_set_ce() modify shared headroom in-place on cloned SKBs (e.g., cloned by tc mirred, bpf_clone_redirect, or packet capture sockets). This can result in silent packet corruption and page cache corruption. Fix this by ensuring that if the target header starts at a negative offset in the headroom, skb_cow(skb, -offset) is called to unshare the headroom before ensuring writability across the header span. Fixes: 86da71b57383 ("net_sched: Introduce skbmod action") Signed-off-by: Muhammad Bilal --- net/sched/act_skbmod.c | 37 ++++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/net/sched/act_skbmod.c b/net/sched/act_skbmod.c index a8e2b83ebae5..cd2a6e974e6f 100644 --- a/net/sched/act_skbmod.c +++ b/net/sched/act_skbmod.c @@ -22,13 +22,24 @@ static struct tc_action_ops act_skbmod_ops; +static int skbmod_ensure_writable(struct sk_buff *skb, int offset, int len) +{ + if (offset < 0) { + if (skb_cow(skb, -offset)) + return -ENOMEM; + if (offset + len > 0) + return skb_ensure_writable(skb, offset + len); + return 0; + } + return skb_ensure_writable(skb, offset + len); +} + TC_INDIRECT_SCOPE int tcf_skbmod_act(struct sk_buff *skb, const struct tc_action *a, struct tcf_result *res) { struct tcf_skbmod *d = to_skbmod(a); struct tcf_skbmod_params *p; - int max_edit_len, err; u64 flags; tcf_lastuse_update(&d->tcf_tm); @@ -38,7 +49,6 @@ TC_INDIRECT_SCOPE int tcf_skbmod_act(struct sk_buff *skb, if (unlikely(p->action == TC_ACT_SHOT)) goto drop; - max_edit_len = skb_mac_header_len(skb); flags = p->flags; /* tcf_skbmod_init() guarantees "flags" to be one of the following: @@ -52,19 +62,20 @@ TC_INDIRECT_SCOPE int tcf_skbmod_act(struct sk_buff *skb, switch (skb_protocol(skb, true)) { case cpu_to_be16(ETH_P_IP): case cpu_to_be16(ETH_P_IPV6): - max_edit_len += skb_network_header_len(skb); + if (skbmod_ensure_writable(skb, skb_network_offset(skb), + skb_network_header_len(skb))) + goto drop; break; default: goto out; } - } else if (!skb->dev || skb->dev->type != ARPHRD_ETHER) { - goto out; - } - - err = skb_ensure_writable(skb, max_edit_len); - if (unlikely(err)) /* best policy is to drop on the floor */ - goto drop; + } else { + if (!skb->dev || skb->dev->type != ARPHRD_ETHER) + goto out; + if (skbmod_ensure_writable(skb, skb_mac_offset(skb), ETH_HLEN)) + goto drop; + } if (flags & SKBMOD_F_DMAC) ether_addr_copy(eth_hdr(skb)->h_dest, p->eth_dst); if (flags & SKBMOD_F_SMAC) -- 2.43.0