[PATCH net] net/sched: act_skbmod: Fix headroom COW leading to page cache corruption

[PATCH net] net/sched: act_skbmod: Fix headroom COW leading to page cache corruption

[Muhammad Bilal]

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 <meatuni001@gmail.com>
---
 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

[PATCH net] net/sched: act_nat: Fix missing headroom COW and integer underflow in header rewriting

[Muhammad Bilal]

tcf_nat_act() accesses and rewrites IPv4, TCP, UDP, and ICMP headers
based on noff = skb_network_offset(skb).

When the network header resides in the headroom (noff < 0), two issues
occur:
1. sizeof(*iph) + noff can evaluate to a negative value or underflow
   when passed to functions expecting unsigned lengths, such as
   pskb_may_pull() and skb_try_make_writable().
2. skb_try_make_writable() only evaluates writability from skb->data
   forwards and does not invoke skb_cow() on the headroom. When modifying
   cloned SKBs (e.g. from packet sockets, tc mirred, or BPF redirects),
   in-place header modification via iph->saddr / iph->daddr mutates
   shared headroom data directly, leading to packet corruption and page
   cache corruption.

Fix this by introducing a helper nat_ensure_writable() that validates
headroom using skb_cow(skb, -offset) when offset is negative before
ensuring writability across the modified header length.

Fixes: b4219952356b ("[PKT_SCHED]: Add stateless NAT")
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
 net/sched/act_nat.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c
index 28cb48419616..1bf5d55b3dc4 100644
--- a/net/sched/act_nat.c
+++ b/net/sched/act_nat.c
@@ -112,6 +112,18 @@ static struct tc_action_ops act_nat_ops;
 
 static const struct rhashtable_params tcf_nat_ht_params;
 
+static int nat_ensure_writable(struct sk_buff *skb, int offset, size_t len)
+{
+	if (offset < 0) {
+		if (skb_cow(skb, -offset))
+			return -ENOMEM;
+		if (offset + (int)len > 0)
+			return skb_ensure_writable(skb, offset + len);
+		return 0;
+	}
+	return skb_ensure_writable(skb, offset + len);
+}
+
 TC_INDIRECT_SCOPE int tcf_nat_act(struct sk_buff *skb,
 				  const struct tc_action *a,
 				  struct tcf_result *res)
@@ -142,7 +154,7 @@ TC_INDIRECT_SCOPE int tcf_nat_act(struct sk_buff *skb,
 	egress = parms->flags & TCA_NAT_FLAG_EGRESS;
 
 	noff = skb_network_offset(skb);
-	if (!pskb_may_pull(skb, sizeof(*iph) + noff))
+	if (nat_ensure_writable(skb, noff, sizeof(*iph)))
 		goto drop;
 
 	iph = ip_hdr(skb);
@@ -153,9 +165,6 @@ TC_INDIRECT_SCOPE int tcf_nat_act(struct sk_buff *skb,
 		addr = iph->daddr;
 
 	if (!((old_addr ^ addr) & mask)) {
-		if (skb_try_make_writable(skb, sizeof(*iph) + noff))
-			goto drop;
-
 		new_addr &= mask;
 		new_addr |= addr & ~mask;
 
@@ -180,8 +189,7 @@ TC_INDIRECT_SCOPE int tcf_nat_act(struct sk_buff *skb,
 	{
 		struct tcphdr *tcph;
 
-		if (!pskb_may_pull(skb, ihl + sizeof(*tcph) + noff) ||
-		    skb_try_make_writable(skb, ihl + sizeof(*tcph) + noff))
+		if (nat_ensure_writable(skb, noff, ihl + sizeof(*tcph)))
 			goto drop;
 
 		tcph = (void *)(skb_network_header(skb) + ihl);
@@ -193,8 +201,7 @@ TC_INDIRECT_SCOPE int tcf_nat_act(struct sk_buff *skb,
 	{
 		struct udphdr *udph;
 
-		if (!pskb_may_pull(skb, ihl + sizeof(*udph) + noff) ||
-		    skb_try_make_writable(skb, ihl + sizeof(*udph) + noff))
+		if (nat_ensure_writable(skb, noff, ihl + sizeof(*udph)))
 			goto drop;
 
 		udph = (void *)(skb_network_header(skb) + ihl);
@@ -209,19 +216,19 @@ TC_INDIRECT_SCOPE int tcf_nat_act(struct sk_buff *skb,
 	{
 		struct icmphdr *icmph;
 
-		if (!pskb_may_pull(skb, ihl + sizeof(*icmph) + noff))
+		if (nat_ensure_writable(skb, noff, ihl + sizeof(*icmph)))
 			goto drop;
 
 		icmph = (void *)(skb_network_header(skb) + ihl);
 
 		if (!icmp_is_err(icmph->type))
 			break;
 
-		if (!pskb_may_pull(skb, ihl + sizeof(*icmph) + sizeof(*iph) +
-					noff))
+		if (nat_ensure_writable(skb, noff,
+					ihl + sizeof(*icmph) + sizeof(*iph)))
 			goto drop;
 
 		icmph = (void *)(skb_network_header(skb) + ihl);
 		iph = (void *)(icmph + 1);
 		if (egress)
 			addr = iph->daddr;
 		else
 			addr = iph->saddr;
 
 		if ((old_addr ^ addr) & mask)
 			break;
-
-		if (skb_try_make_writable(skb, ihl + sizeof(*icmph) +
-					  sizeof(*iph) + noff))
-			goto drop;
 
 		icmph = (void *)(skb_network_header(skb) + ihl);
 		iph = (void *)(icmph + 1);
-- 
2.43.0

[PATCH net] net/sched: act_csum: Fix missing headroom COW and integer underflow in header rewriting

[Muhammad Bilal]

tcf_csum_skb_nextlayer() and tcf_csum_ipv4() access and rewrite IP and L4
headers based on ntkoff = skb_network_offset(skb).

When the network header resides in the headroom (ntkoff < 0):
1. hl + ntkoff or sizeof(*iph) + ntkoff can evaluate to a negative value
   or underflow when passed to functions expecting unsigned lengths, such
   as pskb_may_pull() and skb_try_make_writable().
2. skb_try_make_writable() only evaluates writability from skb->data
   forwards and does not invoke skb_cow() on the headroom. When modifying
   cloned SKBs (e.g. from packet sockets, tc mirred, or BPF redirects),
   updating headers via ip_send_check() or L4 checksum replacements mutates
   shared headroom data directly, leading to packet corruption and page
   cache corruption.

Fix this by introducing a helper csum_ensure_writable() that validates
headroom using skb_cow(skb, -offset) when offset is negative before
ensuring writability across the modified header length.

Fixes: eb4d40654505 ("net/sched: add ACT_CSUM action to update packets checksums")
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
 net/sched/act_csum.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c
index a8e2b83ebae5..cd2a6e974e6f 100644
--- a/net/sched/act_csum.c
+++ b/net/sched/act_csum.c
@@ -124,6 +124,18 @@ static int tcf_csum_init(struct net *net, struct nlattr *nla,
 	return err;
 }
 
+static int csum_ensure_writable(struct sk_buff *skb, int offset, size_t len)
+{
+	if (offset < 0) {
+		if (skb_cow(skb, -offset))
+			return -ENOMEM;
+		if (offset + (int)len > 0)
+			return skb_ensure_writable(skb, offset + len);
+		return 0;
+	}
+	return skb_ensure_writable(skb, offset + len);
+}
+
 /**
  * tcf_csum_skb_nextlayer - Get next layer pointer
  * @skb: sk_buff to use
@@ -139,8 +151,7 @@ static void *tcf_csum_skb_nextlayer(struct sk_buff *skb,
 	int ntkoff = skb_network_offset(skb);
 	int hl = ihl + jhl;
 
-	if (!pskb_may_pull(skb, ipl + ntkoff) || (ipl < hl) ||
-	    skb_try_make_writable(skb, hl + ntkoff))
+	if (ipl < hl || csum_ensure_writable(skb, ntkoff, max_t(unsigned int, ipl, hl)))
 		return NULL;
 	else
 		return (void *)(skb_network_header(skb) + ihl);
@@ -437,8 +448,8 @@ static int tcf_csum_ipv4(struct sk_buff *skb, u32 update_flags)
 	}
 
 	if (update_flags & TCA_CSUM_UPDATE_FLAG_IPV4HDR) {
-		if (skb_try_make_writable(skb, sizeof(*iph) + ntkoff))
+		if (csum_ensure_writable(skb, ntkoff, sizeof(*iph)))
 			goto fail;
 
 		ip_send_check(ip_hdr(skb));
 	}
-- 
2.43.0

Re: [PATCH net] net/sched: act_skbmod: Fix headroom COW leading to page cache corruption

[Jamal Hadi Salim]

On Tue, Aug 18, 2026 at 6:15 PM Muhammad Bilal <meatuni001@gmail.com> wrote:
>
> 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.
>

1. You must provide a reproducer. We test everything. Make sure you
have the correct logs for what you reproduced.
If the reproducer is security-sensitive, send it off-list. A standard
tdc test (if possible) is the prefered option.

2. These bugs follow the same exact pattern; no need for 3 patches.
Fold them into 1. Create a helper they all use.

In case wasnt clear: You need a v2.

cheers,
jamal

> Fixes: 86da71b57383 ("net_sched: Introduce skbmod action")
> Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
> ---
>  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

Re: [PATCH net] net/sched: act_csum: Fix missing headroom COW and integer underflow in header rewriting

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net/sched: act_csum: Fix missing headroom COW and integer underflow in header rewriting

This adds a helper csum_ensure_writable() to act_csum that calls
skb_cow(skb, -offset) when the network offset is negative before calling
skb_ensure_writable(), and converts two of the existing offset
computations in tcf_csum_skb_nextlayer() and tcf_csum_ipv4() to use it.

A few questions below, first about the changelog and then inline in the
diff.

>     2. skb_try_make_writable() only evaluates writability from skb->data
>        forwards and does not invoke skb_cow() on the headroom. When modifying
>        cloned SKBs (e.g. from packet sockets, tc mirred, or BPF redirects),
>        updating headers via ip_send_check() or L4 checksum replacements mutates
>        shared headroom data directly, leading to packet corruption and page
>        cache corruption.

[Severity: Low]
Can this code reach page cache pages at all?  The bytes protected here are
the headroom in front of skb->data, which lives inside the skb head
allocation, and skb_cow() -> __skb_cow() -> pskb_expand_head() only
reallocates and copies skb->head .. skb_tail_pointer().

Page cache pages only show up in skb_shinfo(skb)->frags (sendfile,
MSG_ZEROCOPY), and neither the old nor the new act_csum code writes there.

The "cloned SKBs" framing also looks questionable for the ntkoff >= 0 case.
skb_ensure_writable() falls back to a full head copy:

net/core/skbuff.c:skb_ensure_writable() {
	if (!skb_cloned(skb) || skb_clone_writable(skb, write_len))
		return 0;

	return pskb_expand_head(skb, 0, 0, GFP_ATOMIC);
}

and the fast path only triggers when

include/linux/skbuff.h:skb_clone_writable() {
	return !skb_header_cloned(skb) &&
	       skb_headroom(skb) + len <= skb->hdr_len;
}

i.e. when the header area including headroom was explicitly handed to this
skb as privately writable (see "DOC: dataref and headerless skbs" in
include/linux/skbuff.h).  Could the changelog drop the page cache claim and
describe the headroom case only?

> diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c
> index 078d3a27130b..c8e2f20e2da3 100644
> --- a/net/sched/act_csum.c
> +++ b/net/sched/act_csum.c
> @@ -121,6 +121,18 @@ static int tcf_csum_init(struct net *net, struct nlattr *nla,
>  	return err;
>  }
>  
> +static int csum_ensure_writable(struct sk_buff *skb, int offset, size_t len)
> +{
> +	if (offset < 0) {
> +		if (skb_cow(skb, -offset))
> +			return -ENOMEM;
> +		if (offset + (int)len > 0)
> +			return skb_ensure_writable(skb, offset + len);
> +		return 0;
> +	}
> +	return skb_ensure_writable(skb, offset + len);
> +}

[Severity: High]
Is the (int)len cast here safe?  len is size_t, so any length above INT_MAX
becomes negative in this test, the helper returns 0 without calling
pskb_may_pull() or skb_ensure_writable() at all, and the caller treats the
range as validated.

On the IPv6 path ipl is not bounded.  tcf_csum_ipv6_hopopts() can overwrite
pl with the full 32-bit Jumbo Payload value:

net/sched/act_csum.c:tcf_csum_ipv6_hopopts() {
	...
			*pl = ntohl(*(__be32 *)(xh + off + 2));
			goto done;
	...
}

so with pl = 0xffffffc8 the caller passes len = ipl = 0xfffffff0, where
(int)len == -16 and ntkoff < 0 makes offset + (int)len > 0 false.
tcf_csum_skb_nextlayer() then returns skb_network_header(skb) + ihl and the
L4 handler walks the claimed length:

net/sched/act_csum.c:tcf_csum_ipv6_icmp() {
	...
	skb->csum = csum_partial(icmp6h, ipl - ihl, 0);
	...
}

The removed pskb_may_pull(skb, ipl + ntkoff) rejected this input
(0xfffffff0 + (-20) = 0xffffffdc, pull fails).  Should the helper reject
lengths it cannot represent instead of returning success, and should the
kernel-doc claim "Check the expected next layer availability" still hold?

[Severity: High]
This isn't a bug introduced by this patch, but tcf_csum_sctp() recomputes
the offset independently of this helper and hands it to an unsigned
parameter:

net/sched/act_csum.c:tcf_csum_sctp() {
	...
	sctph->checksum = sctp_compute_cksum(skb,
					     skb_network_offset(skb) + ihl);
	...
}

include/net/sctp/checksum.h:sctp_compute_cksum(struct sk_buff *skb,
						unsigned int offset) {
	struct sctphdr *sh = (struct sctphdr *)(skb->data + offset);
	__le32 old = sh->checksum;
	...
	sh->checksum = 0;
	new = ~skb_crc32c(skb, offset, skb->len - offset, ~0);
	sh->checksum = old;
	...
}

ihl comes from iph->ihl * 4 and tcf_csum_ipv4() never checks iph->ihl >= 5,
so ihl == 0 is packet selectable; with ntkoff == -20 the offset becomes
0xffffffec and both sh->checksum writes land roughly 4 GiB past skb->data.

Since this patch declares negative network offsets supported, and the new
offset + (int)len <= 0 branch admits packets that pskb_may_pull(skb,
ipl + ntkoff) previously rejected, does this widen the set of packets
reaching that write?  Should tcf_csum_sctp() also go through a signed,
bounds-checked path?

>  
>  /**
>   * tcf_csum_skb_nextlayer - Get next layer pointer
>   * @skb: sk_buff to use
> @@ -138,8 +150,7 @@ static void *tcf_csum_skb_nextlayer(struct sk_buff *skb,
>  	int ntkoff = skb_network_offset(skb);
>  	int hl = ihl + jhl;
>  
> -	if (!pskb_may_pull(skb, ipl + ntkoff) || (ipl < hl) ||
> -	    skb_try_make_writable(skb, hl + ntkoff))
> +	if (ipl < hl || csum_ensure_writable(skb, ntkoff, max_t(unsigned int, ipl, hl)))
>  		return NULL;
>  	else
>  		return (void *)(skb_network_header(skb) + ihl);

[Severity: Medium]
Is the change of the requested writable length intentional here?  The old
code asked for hl + ntkoff (mac_len plus IP plus L4 header); the new code
asks for ntkoff + ipl, the whole packet length taken from iph->tot_len or
the IPv6 payload length.

Because the ipl < hl test short-circuits first, ipl >= hl always holds when
the helper runs, so max_t(unsigned int, ipl, hl) is always ipl and the
max_t() looks like dead code.

The widened length also disables the clone fast path:

include/linux/skbuff.h:skb_clone_writable() {
	return !skb_header_cloned(skb) &&
	       skb_headroom(skb) + len <= skb->hdr_len;
}

For transport clones (__skb_header_release() plus skb_clone(), i.e. every
non-GSO TCP transmit skb) hdr_len - headroom covers only the mac plus IP
plus TCP bytes pushed after cloning, which satisfied hl + ntkoff but never
satisfies ntkoff + tot_len.  Those skbs now take
pskb_expand_head(skb, 0, 0, GFP_ATOMIC) in skb_ensure_writable() per
packet, and when that allocation fails the action drops a packet that was
previously forwarded without allocating.

The callers only write inside the L4 header, so would hl be the right
length to request, and should the changelog mention the per-packet copy?

> @@ -437,7 +448,7 @@ static int tcf_csum_ipv4(struct sk_buff *skb, u32 update_flags)
>  	}
>  
>  	if (update_flags & TCA_CSUM_UPDATE_FLAG_IPV4HDR) {
> -		if (skb_try_make_writable(skb, sizeof(*iph) + ntkoff))
> +		if (csum_ensure_writable(skb, ntkoff, sizeof(*iph)))
>  			goto fail;
>  
>  		ip_send_check(ip_hdr(skb));

[Severity: Medium]
Can this new call ever run for the negative offsets the changelog talks
about?  Earlier in the same function the entry gate is untouched:

net/sched/act_csum.c:tcf_csum_ipv4() {
	ntkoff = skb_network_offset(skb);

	if (!pskb_may_pull(skb, sizeof(*iph) + ntkoff))
		goto fail;
	...
}

sizeof() is size_t, so a negative ntkoff is converted: the sum is exactly 0
for ntkoff == -20 (passes) and wraps for ntkoff <= -21 (pull fails).  The
new headroom COW in the IPV4HDR branch is therefore unreachable for the
deeply negative offsets.

The changelog names "sizeof(*iph) + ntkoff" as one of the expressions being
fixed, but that expression is left in place.

tcf_csum_ipv6() also keeps four such computations:

net/sched/act_csum.c:tcf_csum_ipv6() {
	if (!pskb_may_pull(skb, hl + ntkoff))
		goto fail;
	...
			if (!pskb_may_pull(skb, hl + sizeof(*ip6xh) + ntkoff))
	...
			if (!pskb_may_pull(skb, hl + ixhl + ntkoff))
	...
	} while (pskb_may_pull(skb, hl + 1 + ntkoff));
}

so the IPv6 half still rejects negative offsets while feeding the now
negative-offset-tolerant tcf_csum_skb_nextlayer(), and tcf_csum_sctp()
still passes skb_network_offset(skb) + ihl unsigned.  Should negative
ntkoff be handled in all of these places, or rejected once at the top of
both tcf_csum_ipv4() and tcf_csum_ipv6()?
-- 
pw-bot: cr