Re: [PATCH net] net/sched: fix pedit partial COW leading to page cache corruption
From: David Laight <hidden>
Date: 2026-05-26 09:48:25
On Sat, 23 May 2026 12:57:08 -0400 Jamal Hadi Salim [off-list ref] wrote:
quoted hunk ↗ jump to hunk
On Sat, May 23, 2026 at 12:46 PM Jakub Kicinski [off-list ref] wrote:quoted
On Sat, 23 May 2026 08:13:21 -0400 Jamal Hadi Salim wrote:quoted
quoted
quoted
@@ -474,8 +473,6 @@ TC_INDIRECT_SCOPE int tcf_pedit_act(struct sk_buff *skb, } *ptr = ((*ptr & tkey->mask) ^ val); - if (ptr == &hdata) - skb_store_bits(skb, hoffset + offset, ptr, 4); } goto done;I see you are trying to get rid of the skb_header_pointer() / skb_store_bits() piece. Sure looks cleaner if we must linearize.The other thing (i may be over thinking) with pskb_may_pull is: if the data is already linear (in a clone), wouldn't we corrupt the shared linear data of the clone?I said for the portion of the problem that's "we are writing to frags" IOW not replacing the rest of the patch (assuming we care).So as an alternative to the piece i posted? i.e this:diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c index 79921b8d89ba..8f0f84b50c85 100644 --- a/net/sched/act_pedit.c +++ b/net/sched/act_pedit.c@@ -474,6 +474,12 @@ TC_INDIRECT_SCOPE int tcf_pedit_act(struct sk_buff *skb, if (write_offset < 0) { if (skb_cow(skb, -write_offset)) goto bad; + if (write_offset + (int)sizeof(hdata) > 0) { + if (skb_ensure_writable(skb, + min_t(int, skb->len, + write_offset +(int)sizeof(hdata))))
I don't think that needs to be min_t(), a plain min() will do. (Possibly with the (int) cast removed from the sizeof.) -- David
+ goto bad;
+ }
} else {
if (unlikely(check_add_overflow(write_offset,
(int)sizeof(hdata),
cheers,
jamal