[Cake] [PATCH net v4 0/2] net/sched: sch_cake: prevent shaper corruption and stall in cake_overhead()

[Cake] [PATCH net v4 0/2] net/sched: sch_cake: prevent shaper corruption and stall in cake_overhead()

[Yuchao Zhang]

This series addresses two issues in cake_overhead() that can lead to
corrupted rate shaper accounting or long dequeue stalls:

Patch 1 fixes an integer underflow when segs == 0. When an skb with
segs == 0 (such as dodgy GSO packets where gso_segs is not recomputed)
reaches cake_overhead(), (segs - 1) wraps around to UINT32_MAX,
multiplying per-segment overhead by ~4.29 billion and returning a length
close to 4.29 GB. cake_advance_shaper() then charges that length to the
shaper, stalling the dequeue queue for tens of seconds at 1 Gbit/s, and
for minutes to hours at lower rates. This was introduced by commit
c5d34f4583ea ("net_sched: cake: use qdisc_pkt_segs()").

Patch 2 validates the transport header offset computed in cake_overhead().
When the transport header was never set, skb_transport_offset() returns
the ~0U sentinel (~65535), which inflates shaper accounting by ~66 KB
per segment. Furthermore, if preceding egress BPF filters (e.g.
sch_handle_egress()) or cake classifier actions (e.g. act_bpf trimming
headers via bpf_skb_adjust_room(BPF_ADJ_ROOM_MAC)) leave the transport
header stale (bpf_skb_net_hdr_pop() only re-syncs it when it aliased
network_header), skb_transport_offset() can become negative. Because
hdr_len was declared as unsigned int, a negative offset wraps to near
UINT_MAX. Patch 2 checks !skb_transport_header_was_set() and ensures
hdr_len >= 0, unifying all early fallback exits to an 'err' label at the
end of the function. Both hunks date back to commit a729b7f0bd5b
("sch_cake: Add overhead compensation support to the rate shaper").

Changes in v4:
  - Patch 1: Add Acked-by: Toke Høiland-Jørgensen <toke@toke.dk>.
  - Patch 2: Consolidate the 3 duplicate calls to cake_calc_overhead() into
    a single 'err' label at the end of the function, turning early returns
    into 'goto err' statements per Toke Høiland-Jørgensen review.
  - Link to v3: https://lore.kernel.org/netdev/20260927131009.24250-1-ndaugoing@gmail.com/

Changes in v3:
  - Split the v2 patch into a 2-patch series per Simon Horman and Sashiko
    AI review so each logical fix carries its own accurate Fixes: tag and
    matches proper stable tree backport ranges:
    - Patch 1 Fixes: c5d34f4583ea ("net_sched: cake: use qdisc_pkt_segs()")
    - Patch 2 Fixes: a729b7f0bd5b ("sch_cake: Add overhead
      compensation support to the rate shaper")
  - Clarify the timing and code paths where header mangling can occur
    (sch_handle_egress() and cake_classify() before cake_overhead()) rather
    than inaccurate "post-enqueue mangling" wording.
  - Link to v2: https://lore.kernel.org/netdev/20260922084124.36858-1-ndaugoing@gmail.com/

Changes in v2:
  - Accurately describe the impact as shaper accounting corruption / stall
    rather than OOB read past the allocation.
  - Fix integer underflow when segs == 0 by checking segs <= 1.
  - Import companion check !skb_transport_header_was_set(skb) from
    qdisc_pkt_len_segs_init() to prevent unset transport header sentinel
    (~0U) from inflating packet length to ~66 KB.
  - Link to v1: https://lore.kernel.org/netdev/20260917122153.62722-1-ndaugoing@gmail.com/

Yuchao Zhang (2):
  net/sched: sch_cake: fix shaper stall on segs == 0 in cake_overhead()
  net/sched: sch_cake: validate transport header offset in
    cake_overhead()

 net/sched/sch_cake.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

-- 
2.53.0

[Cake] [PATCH net v4 1/2] net/sched: sch_cake: fix shaper stall on segs == 0 in cake_overhead()

[Yuchao Zhang]

In cake_overhead(), packets with a single segment bypass multi-segment
overhead calculations:

	if (segs == 1)
		return cake_calc_overhead(q, len, off);

Commit c5d34f4583ea ("net_sched: cake: use qdisc_pkt_segs()") switched
cake to retrieve the cached segmentation count via qdisc_pkt_segs(skb)
instead of calculating it locally for dodgy GSO packets. If an skb with
segs == 0 reaches cake_overhead(), it skips the segs == 1 early return
and enters the multi-segment arithmetic:

	len = shinfo->gso_size + hdr_len;
	last_len = skb->len - shinfo->gso_size * (segs - 1);

	return (cake_calc_overhead(q, len, off) * (segs - 1) +
		cake_calc_overhead(q, last_len, off));

Because segs is an unsigned 16-bit integer, (segs - 1) underflows to
65535 (and is promoted to 4294967295 in the 32-bit unsigned arithmetic
above). This multiplies the per-segment overhead by UINT32_MAX, so
cake_overhead() returns a length close to 4.29 GB. cake_advance_shaper()
then charges that length to the shaper, stalling the CAKE dequeue path
for tens of seconds at 1 Gbit/s, and for minutes to hours at lower rates.

Fix this by returning early with cake_calc_overhead(q, len, off) whenever
segs <= 1.

Fixes: c5d34f4583ea ("net_sched: cake: use qdisc_pkt_segs()")
Cc: stable@vger.kernel.org
Acked-by: Toke Høiland-Jørgensen <toke@toke.dk>
Signed-off-by: Yuchao Zhang <ndaugoing@gmail.com>
---
v4:
 - Add Acked-by: Toke Høiland-Jørgensen <toke@toke.dk>.
 - Link to v3: https://lore.kernel.org/netdev/20260927131009.24250-1-ndaugoing@gmail.com/

v3:
 - Split from v2 into a standalone patch with its own Fixes: tag
   (c5d34f4583ea) per Simon Horman and Sashiko review.
 - Link to v2: https://lore.kernel.org/netdev/20260922084124.36858-1-ndaugoing@gmail.com/
 - Link to v1: https://lore.kernel.org/netdev/20260917122153.62722-1-ndaugoing@gmail.com/
 net/sched/sch_cake.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c
index dc93267029e7..b0d604a7052a 100644
--- a/net/sched/sch_cake.c
+++ b/net/sched/sch_cake.c
@@ -1420,7 +1420,7 @@ static u32 cake_overhead(struct cake_sched_data *q, const struct sk_buff *skb)
 
 	WRITE_ONCE(q->avg_netoff, cake_ewma(q->avg_netoff, off << 16, 8));
 
-	if (segs == 1)
+	if (segs <= 1)
 		return cake_calc_overhead(q, len, off);
 
 	/* borrowed from qdisc_pkt_len_segs_init() */
-- 
2.53.0

[Cake] Re: [PATCH net v4 1/2] net/sched: sch_cake: fix shaper stall on segs == 0 in cake_overhead()

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit message's explanation of the failure is wrong, and it
  leaves out what actually triggers the stall.

--

net/sched: sch_cake: fix shaper stall on segs == 0 in cake_overhead()

This patch changes the single-segment early return in cake_overhead() from
segs == 1 to segs <= 1. A GSO skb whose cached segment count is 0 then
skips the multi-segment overhead arithmetic, which could charge a huge
length to the shaper.

> Because segs is an unsigned 16-bit integer, (segs - 1) underflows to
> 65535 (and is promoted to 4294967295 in the 32-bit unsigned arithmetic
> above). This multiplies the per-segment overhead by UINT32_MAX, so
> cake_overhead() returns a length close to 4.29 GB.

[Severity: Low]
Is this description of the arithmetic accurate?

segs is a u16, so (segs - 1) is promoted to int and evaluates to -1. It
never becomes 65535. It only turns into 0xffffffff when it is multiplied
by the u32 that cake_calc_overhead() returns.

The description also leaves out the last-segment term. With segs == 0:

    len = shinfo->gso_size + hdr_len;
    last_len = skb->len - shinfo->gso_size * (segs - 1);

last_len becomes skb->len + gso_size, so the old return value was in
effect:

    calc(skb->len + gso_size) - calc(gso_size + hdr_len)   (mod 2^32)

cake_calc_overhead() never decreases as len grows. So whenever
hdr_len <= skb->len, the result is small, roughly the payload size and a
slight undercharge, and the shaper does not stall.

The ~4.29 GB charge only happens when hdr_len > skb->len. In practice that
means a non-encapsulated skb whose transport header was never set. In that
case skb_transport_offset() is around 65300 and skb_header_pointer() fails.

That is the same case where qdisc_pkt_len_segs_init() returns early and
leaves pkt_segs at the raw gso_segs value, which can be 0:

net/core/dev.c:qdisc_pkt_len_segs_init() {
    ...
	qdisc_skb_cb(skb)->pkt_segs = gso_segs = shinfo->gso_segs;
    ...
	if (!skb->encapsulation) {
		if (unlikely(!skb_transport_header_was_set(skb)))
			return SKB_NOT_DROPPED_YET;
    ...
}

When the transport header is set, the later pskb_may_pull() calls in
qdisc_pkt_len_segs_init() guarantee hdr_len <= skb->len for TCP and
UDP_L4. For SKB_GSO_DODGY, gso_segs is recomputed with DIV_ROUND_UP() and
ends up at 1 or more. So an skb with segs == 0 and a valid transport header
does not stall the shaper.

The code change itself looks correct. Could the commit message correct the
promotion explanation? Could it also state that an unset transport header
is the precondition for the stall, so stable backporters can judge their
exposure?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260928132452.59996-1-ndaugoing%40gmail.com

[Cake] [PATCH net v4 2/2] net/sched: sch_cake: validate transport header offset in cake_overhead()

[Yuchao Zhang]

In cake_overhead(), the header length up to the transport layer is
computed using logic borrowed from qdisc_pkt_len_segs_init():

	/* borrowed from qdisc_pkt_len_segs_init() */
	if (!skb->encapsulation)
		hdr_len = skb_transport_offset(skb);
	else
		hdr_len = skb_inner_transport_offset(skb);

However, cake_overhead() does not validate the computed offset:

1. When the transport header was never set, skb->transport_header holds
   the sentinel value ~0U. skb_transport_offset() returns ~65535.
   skb_header_pointer() subsequently fails, leaving hdr_len as ~65535,
   charging ~66 KB per segment to the shaper.
   Mirror qdisc_pkt_len_segs_init() by bailing out when
   unlikely(!skb_transport_header_was_set(skb)).

2. While qdisc_pkt_len_segs_init() runs at the start of __dev_queue_xmit(),
   packet headers may be adjusted before cake_overhead() is reached:
   - in sch_handle_egress() via tc/BPF egress filters;
   - inside cake_enqueue() via cake_classify() -> tcf_classify() (e.g.
     act_bpf, act_pedit, act_mpls, act_vlan).
   For example, bpf_skb_adjust_room(..., BPF_ADJ_ROOM_MAC) invokes
   bpf_skb_net_hdr_pop(), which pulls skb->data forward and re-syncs
   transport_header only when it aliased network_header, i.e. when no
   transport header had been parsed. If a transport header had been
   parsed, its offset is left where it was while skb->data moves forward,
   so skb_transport_offset() becomes old_offset - len and can turn
   negative. Since hdr_len was declared as unsigned int, a negative offset
   wraps around to near UINT_MAX, corrupting header length accounting.

Declare hdr_len as int and fall back to cake_calc_overhead(q, len, off) if
unlikely(hdr_len < 0). Consolidate all early fallback paths into a single
'err' label at the end of the function.

Fixes: a729b7f0bd5b ("sch_cake: Add overhead compensation support to the rate shaper")
Cc: stable@vger.kernel.org
Signed-off-by: Yuchao Zhang <ndaugoing@gmail.com>
---
v4:
 - Consolidate the 3 duplicate calls to cake_calc_overhead() into a single
   'err' label at the end of the function, turning early returns into
   'goto err' statements per Toke Høiland-Jørgensen review.
 - Link to v3: https://lore.kernel.org/netdev/20260927131009.24250-1-ndaugoing@gmail.com/

v3:
 - Split from v2 into a standalone patch with its own Fixes: tag
   (a729b7f0bd5b) per Simon Horman and Sashiko review.
 - Clarify header mangling ordering (sch_handle_egress() and cake_classify()
   before cake_overhead()) rather than inaccurate "post-enqueue mangling"
   wording per Sashiko review.
 - Link to v2: https://lore.kernel.org/netdev/20260922084124.36858-1-ndaugoing@gmail.com/
 - Link to v1: https://lore.kernel.org/netdev/20260917122153.62722-1-ndaugoing@gmail.com/
 net/sched/sch_cake.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c
index b0d604a7052a..f8deb8083f16 100644
--- a/net/sched/sch_cake.c
+++ b/net/sched/sch_cake.c
@@ -1413,21 +1413,28 @@ static u32 cake_calc_overhead(struct cake_sched_data *qd, u32 len, u32 off)
 static u32 cake_overhead(struct cake_sched_data *q, const struct sk_buff *skb)
 {
 	const struct skb_shared_info *shinfo = skb_shinfo(skb);
-	unsigned int hdr_len, last_len = 0;
+	unsigned int last_len = 0;
 	u32 off = skb_network_offset(skb);
 	u16 segs = qdisc_pkt_segs(skb);
 	u32 len = qdisc_pkt_len(skb);
+	int hdr_len;
 
 	WRITE_ONCE(q->avg_netoff, cake_ewma(q->avg_netoff, off << 16, 8));
 
 	if (segs <= 1)
-		return cake_calc_overhead(q, len, off);
+		goto err;
 
 	/* borrowed from qdisc_pkt_len_segs_init() */
-	if (!skb->encapsulation)
+	if (!skb->encapsulation) {
+		if (unlikely(!skb_transport_header_was_set(skb)))
+			goto err;
 		hdr_len = skb_transport_offset(skb);
-	else
+	} else {
 		hdr_len = skb_inner_transport_offset(skb);
+	}
+
+	if (unlikely(hdr_len < 0))
+		goto err;
 
 	/* + transport layer */
 	if (likely(shinfo->gso_type & (SKB_GSO_TCPV4 |
@@ -1452,6 +1459,9 @@ static u32 cake_overhead(struct cake_sched_data *q, const struct sk_buff *skb)
 
 	return (cake_calc_overhead(q, len, off) * (segs - 1) +
 		cake_calc_overhead(q, last_len, off));
+
+err:
+	return cake_calc_overhead(q, len, off);
 }
 
 static void cake_heap_swap(struct cake_sched_data *q, u16 i, u16 j)
-- 
2.53.0

[Cake] Re: [PATCH net v4 2/2] net/sched: sch_cake: validate transport header offset in cake_overhead()

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · Low: 0 (1)

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The patch is incomplete.

Pre-existing issues:
- [Low] [Baseline: discarded as pre-existing, low] This problem wasn't
  introduced by this patch, but the cake_overhead() block commented…

--

net/sched: sch_cake: validate transport header offset in cake_overhead()

This patch makes cake_overhead() fall back to the plain
cake_calc_overhead() path in two cases: when the transport header was
never set on a non-encapsulated skb, and when the computed transport
offset is negative. hdr_len changes from unsigned int to int so the
negative case can be detected, and all fallback paths now go through a
single err label.

> diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c
> index b0d604a7052a..f8deb8083f16 100644
> --- a/net/sched/sch_cake.c
> +++ b/net/sched/sch_cake.c
> @@ -1413,21 +1413,28 @@ static u32 cake_calc_overhead(struct cake_sched_data *qd, u32 len, u32 off)
>  static u32 cake_overhead(struct cake_sched_data *q, const struct sk_buff *skb)
>  {

[ ... ]

>  	/* borrowed from qdisc_pkt_len_segs_init() */
> -	if (!skb->encapsulation)
> +	if (!skb->encapsulation) {
> +		if (unlikely(!skb_transport_header_was_set(skb)))
> +			goto err;
>  		hdr_len = skb_transport_offset(skb);
> -	else
> +	} else {
>  		hdr_len = skb_inner_transport_offset(skb);
> +	}
> +
> +	if (unlikely(hdr_len < 0))
> +		goto err;

[Severity: High]
Does this fully remove the ~66 KB per-segment charge the commit message
describes? Two paths still seem to reach it.

First, the encapsulated branch has no check for the unset sentinel.
bpf_skb_net_grow() is reached through bpf_skb_adjust_room() with
BPF_F_ADJ_ROOM_ENCAP_L3_*. It copies transport_header into
inner_transport_header without checking whether it was set:

net/core/filter.c:bpf_skb_net_grow() {
    ...
		inner_trans = skb->transport_header;
    ...
		skb->inner_transport_header = inner_trans;
    ...
}

It also sets skb->encapsulation = 1.

qdisc_pkt_len_segs_init() stores pkt_segs before its !was_set early
return:

	qdisc_skb_cb(skb)->pkt_segs = gso_segs = shinfo->gso_segs;
	...
	if (!skb->encapsulation) {
		if (unlikely(!skb_transport_header_was_set(skb)))
			return SKB_NOT_DROPPED_YET;

So a GSO skb with an unset transport header keeps pkt_segs > 1. A tc
egress program in sch_handle_egress(), or act_bpf run from
cake_classify(), can then encapsulate that skb.

cake_overhead() then takes the else branch and computes
hdr_len = 0xFFFF - (skb->data - skb->head). That value is large and
positive, so the hdr_len < 0 test does not catch it.

Second, both the TCP and UDP branches ignore a NULL return from
skb_header_pointer() and keep the offset:

		th = skb_header_pointer(skb, hdr_len,
					sizeof(_tcphdr), &_tcphdr);
		if (likely(th))
			hdr_len += __tcp_hdrlen(th);
	...
	len = shinfo->gso_size + hdr_len;

The unreadable hdr_len goes into len, which is then multiplied by
(segs - 1). In cake_enqueue() the result becomes
get_cobalt_cb(skb)->adjusted_len. That pushes the shaper clock far ahead,
stalls dequeue, and corrupts max_netlen/max_adjlen.

qdisc_pkt_len_segs_init() returns SKB_DROP_REASON_SKB_BAD_GSO whenever
pskb_may_pull() of the transport header fails, which covers both cases.

Would adding goto err when skb_header_pointer() returns NULL, or checking
hdr_len against skb->len, cover the remaining cases on top of the new
checks?

>  
>  	/* + transport layer */
>  	if (likely(shinfo->gso_type & (SKB_GSO_TCPV4 |

[Severity: Low]
This is a pre-existing issue and was not introduced by this patch. The
block labelled "borrowed from qdisc_pkt_len_segs_init()" differs from its
source.

It adds __tcp_hdrlen(th) without the tlen check that
qdisc_pkt_len_segs_init() does:

		tlen = __tcp_hdrlen(th);
		if (tlen < sizeof(*th))
			return SKB_DROP_REASON_SKB_BAD_GSO;

It also adds sizeof(struct udphdr) in a bare else for every non-TCP GSO
type:

	} else {
		struct udphdr _udphdr;

		if (skb_header_pointer(skb, hdr_len,
				       sizeof(_udphdr), &_udphdr))
			hdr_len += sizeof(struct udphdr);
	}

The reference only does this for SKB_GSO_UDP_L4. As a result, SCTP, ESP
or GRE GSO traffic is charged 8 extra bytes per segment. A garbage doff
after header mangling can cause an error of up to about 60 bytes per
segment.

Should this block also apply the SKB_GSO_UDP_L4 and tlen checks that
qdisc_pkt_len_segs_init() uses?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260928132452.59996-1-ndaugoing%40gmail.com

[Cake] Re: [PATCH net v4 0/2] net/sched: sch_cake: prevent shaper corruption and stall in cake_overhead()

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.