[PATCH] net/sched: act_pedit: require matching IPv4 L4 protocol

[PATCH] net/sched: act_pedit: require matching IPv4 L4 protocol

[Samuel Moelius]

The extended IPv4 L4 header mode in act_pedit can select TCP or UDP
header fields without confirming that the IPv4 protocol field matches
the selected transport header.

That lets a rule written for TCP or UDP modify unrelated payload bytes
in a packet carrying a different protocol.

Verify the IPv4 protocol before applying TCP or UDP extended header
edits.

Assisted-by: Codex:gpt-5.5-cyber-preview
Signed-off-by: Samuel Moelius <sam.moelius@trailofbits.com>
---
 net/sched/act_pedit.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index bc20f08a2789..9a4590451f7e 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -341,6 +341,8 @@ static int pedit_l4_skb_offset(struct sk_buff *skb, int *hoffset, const int head
 
 		if (!iph)
 			goto out;
+		if (iph->ihl < 5 || iph->protocol != header_type)
+			goto out;
 		*hoffset = noff + iph->ihl * 4;
 		ret = 0;
 		break;
-- 
2.43.0

Re: [PATCH] net/sched: act_pedit: require matching IPv4 L4 protocol

[Jamal Hadi Salim]

On Fri, Jun 5, 2026 at 3:46 PM Samuel Moelius
<sam.moelius@trailofbits.com> wrote:
>
> The extended IPv4 L4 header mode in act_pedit can select TCP or UDP
> header fields without confirming that the IPv4 protocol field matches
> the selected transport header.
>
> That lets a rule written for TCP or UDP modify unrelated payload bytes
> in a packet carrying a different protocol.
>
> Verify the IPv4 protocol before applying TCP or UDP extended header
> edits.
>
> Assisted-by: Codex:gpt-5.5-cyber-preview
> Signed-off-by: Samuel Moelius <sam.moelius@trailofbits.com>
> ---
>  net/sched/act_pedit.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
> index bc20f08a2789..9a4590451f7e 100644
> --- a/net/sched/act_pedit.c
> +++ b/net/sched/act_pedit.c
> @@ -341,6 +341,8 @@ static int pedit_l4_skb_offset(struct sk_buff *skb, int *hoffset, const int head
>
>                 if (!iph)
>                         goto out;
> +               if (iph->ihl < 5 || iph->protocol != header_type)
> +                       goto out;

From inspection the fix looks reasonable.
At first glance it seems that the only fix that resolves the issue you
are describing is to check the protocol header.
header length seems to be an extra thing. But let's do what the v6
side seems to and skip frags as well? Something maybe along the lines
of:

if (iph->ihl < 5 || iph->protocol != header_type || (iph->frag_off &
htons(IP_OFFSET)))
        goto out;

Are you able to describe how someone would configure a tc rule that
will allow this to happen?
IOW, how did you verify the problem and then validate that your fix works?

cheers,
jamal

>                 *hoffset = noff + iph->ihl * 4;
>                 ret = 0;
>                 break;
> --
> 2.43.0
>

Re: [PATCH] net/sched: act_pedit: require matching IPv4 L4 protocol

[Samuel Moelius]

On Sat, Jun 6, 2026 at 5:29 PM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> On Fri, Jun 5, 2026 at 3:46 PM Samuel Moelius
> <sam.moelius@trailofbits.com> wrote:
> >
> > The extended IPv4 L4 header mode in act_pedit can select TCP or UDP
> > header fields without confirming that the IPv4 protocol field matches
> > the selected transport header.
> >
> > That lets a rule written for TCP or UDP modify unrelated payload bytes
> > in a packet carrying a different protocol.
> >
> > Verify the IPv4 protocol before applying TCP or UDP extended header
> > edits.
> >
> > Assisted-by: Codex:gpt-5.5-cyber-preview
> > Signed-off-by: Samuel Moelius <sam.moelius@trailofbits.com>
> > ---
> >  net/sched/act_pedit.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
> > index bc20f08a2789..9a4590451f7e 100644
> > --- a/net/sched/act_pedit.c
> > +++ b/net/sched/act_pedit.c
> > @@ -341,6 +341,8 @@ static int pedit_l4_skb_offset(struct sk_buff *skb, int *hoffset, const int head
> >
> >                 if (!iph)
> >                         goto out;
> > +               if (iph->ihl < 5 || iph->protocol != header_type)
> > +                       goto out;
>
> From inspection the fix looks reasonable.
> At first glance it seems that the only fix that resolves the issue you
> are describing is to check the protocol header.
> header length seems to be an extra thing. But let's do what the v6
> side seems to and skip frags as well? Something maybe along the lines
> of:
>
> if (iph->ihl < 5 || iph->protocol != header_type || (iph->frag_off &
> htons(IP_OFFSET)))
>         goto out;

I will submit an updated patch.

> Are you able to describe how someone would configure a tc rule that
> will allow this to happen?
> IOW, how did you verify the problem and then validate that your fix works?

The bug was validated using a custom init script in QEMU. The specific
line used was:

/usr/sbin/tc filter add dev veth0 egress protocol ip pref 1 matchall
action pedit ex munge udp dport set 18053

An IPv4 TCP packet with destination port 2222 was sent through that
path. With the unpatched kernel, the packet was received with TCP
destination port 18053. With the patched kernel, the same packet was
received unchanged.

I can provide more details or artifacts if you would like.

Re: [PATCH] net/sched: act_pedit: require matching IPv4 L4 protocol

[Jamal Hadi Salim]

On Sun, Jun 7, 2026 at 10:33 AM Samuel Moelius
<sam.moelius@trailofbits.com> wrote:
>
> On Sat, Jun 6, 2026 at 5:29 PM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> >
> > On Fri, Jun 5, 2026 at 3:46 PM Samuel Moelius
> > <sam.moelius@trailofbits.com> wrote:
> > >
> > > The extended IPv4 L4 header mode in act_pedit can select TCP or UDP
> > > header fields without confirming that the IPv4 protocol field matches
> > > the selected transport header.
> > >
> > > That lets a rule written for TCP or UDP modify unrelated payload bytes
> > > in a packet carrying a different protocol.
> > >
> > > Verify the IPv4 protocol before applying TCP or UDP extended header
> > > edits.
> > >
> > > Assisted-by: Codex:gpt-5.5-cyber-preview
> > > Signed-off-by: Samuel Moelius <sam.moelius@trailofbits.com>
> > > ---
> > >  net/sched/act_pedit.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
> > > index bc20f08a2789..9a4590451f7e 100644
> > > --- a/net/sched/act_pedit.c
> > > +++ b/net/sched/act_pedit.c
> > > @@ -341,6 +341,8 @@ static int pedit_l4_skb_offset(struct sk_buff *skb, int *hoffset, const int head
> > >
> > >                 if (!iph)
> > >                         goto out;
> > > +               if (iph->ihl < 5 || iph->protocol != header_type)
> > > +                       goto out;
> >
> > From inspection the fix looks reasonable.
> > At first glance it seems that the only fix that resolves the issue you
> > are describing is to check the protocol header.
> > header length seems to be an extra thing. But let's do what the v6
> > side seems to and skip frags as well? Something maybe along the lines
> > of:
> >
> > if (iph->ihl < 5 || iph->protocol != header_type || (iph->frag_off &
> > htons(IP_OFFSET)))
> >         goto out;
>
> I will submit an updated patch.
>
> > Are you able to describe how someone would configure a tc rule that
> > will allow this to happen?
> > IOW, how did you verify the problem and then validate that your fix works?
>
> The bug was validated using a custom init script in QEMU. The specific
> line used was:
>
> /usr/sbin/tc filter add dev veth0 egress protocol ip pref 1 matchall
> action pedit ex munge udp dport set 18053
>
> An IPv4 TCP packet with destination port 2222 was sent through that
> path. With the unpatched kernel, the packet was received with TCP
> destination port 18053. With the patched kernel, the same packet was
> received unchanged.
>
> I can provide more details or artifacts if you would like.

A tdc test case is better. Victor or I can create one for you if you
dont know how.

cheers,
jamal

Re: [PATCH] net/sched: act_pedit: require matching IPv4 L4 protocol

[Victor Nogueira]

[-- Attachment #1: Type: text/plain, Size: 3210 bytes --]

On Mon, Jun 8, 2026 at 7:46 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> On Sun, Jun 7, 2026 at 10:33 AM Samuel Moelius
> <sam.moelius@trailofbits.com> wrote:
> >
> > On Sat, Jun 6, 2026 at 5:29 PM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> > >
> > > On Fri, Jun 5, 2026 at 3:46 PM Samuel Moelius
> > > <sam.moelius@trailofbits.com> wrote:
> > > >
> > > > The extended IPv4 L4 header mode in act_pedit can select TCP or UDP
> > > > header fields without confirming that the IPv4 protocol field matches
> > > > the selected transport header.
> > > >
> > > > That lets a rule written for TCP or UDP modify unrelated payload bytes
> > > > in a packet carrying a different protocol.
> > > >
> > > > Verify the IPv4 protocol before applying TCP or UDP extended header
> > > > edits.
> > > >
> > > > Assisted-by: Codex:gpt-5.5-cyber-preview
> > > > Signed-off-by: Samuel Moelius <sam.moelius@trailofbits.com>
> > > > ---
> > > >  net/sched/act_pedit.c | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
> > > > index bc20f08a2789..9a4590451f7e 100644
> > > > --- a/net/sched/act_pedit.c
> > > > +++ b/net/sched/act_pedit.c
> > > > @@ -341,6 +341,8 @@ static int pedit_l4_skb_offset(struct sk_buff *skb, int *hoffset, const int head
> > > >
> > > >                 if (!iph)
> > > >                         goto out;
> > > > +               if (iph->ihl < 5 || iph->protocol != header_type)
> > > > +                       goto out;
> > >
> > > From inspection the fix looks reasonable.
> > > At first glance it seems that the only fix that resolves the issue you
> > > are describing is to check the protocol header.
> > > header length seems to be an extra thing. But let's do what the v6
> > > side seems to and skip frags as well? Something maybe along the lines
> > > of:
> > >
> > > if (iph->ihl < 5 || iph->protocol != header_type || (iph->frag_off &
> > > htons(IP_OFFSET)))
> > >         goto out;
> >
> > I will submit an updated patch.
> >
> > > Are you able to describe how someone would configure a tc rule that
> > > will allow this to happen?
> > > IOW, how did you verify the problem and then validate that your fix works?
> >
> > The bug was validated using a custom init script in QEMU. The specific
> > line used was:
> >
> > /usr/sbin/tc filter add dev veth0 egress protocol ip pref 1 matchall
> > action pedit ex munge udp dport set 18053
> >
> > An IPv4 TCP packet with destination port 2222 was sent through that
> > path. With the unpatched kernel, the packet was received with TCP
> > destination port 18053. With the patched kernel, the same packet was
> > received unchanged.
> >
> > I can provide more details or artifacts if you would like.
>
> A tdc test case is better. Victor or I can create one for you if you
> dont know how.

It seems like v2 isn't applying so you'll have to send a new version.
When you do, after waiting for the 24h period, can you resend with
the attached patch (containing the tdc test)? Since this is a
fix, in the new version, remember to target net and add a proper fixes
tag.

cheers,
Victor

[-- Attachment #2: 0001-selftests-tc-testing-Verify-pedit-does-not-mangle-mi.patch --]
[-- Type: text/x-patch, Size: 3031 bytes --]

From 9fc039564addac0312ceb9c9210cb93ea44e8a9f Mon Sep 17 00:00:00 2001
From: Victor Nogueira <victor@mojatatu.com>
Date: Mon, 8 Jun 2026 11:10:05 -0300
Subject: [PATCH net v3] selftests/tc-testing: Verify pedit does not mangle
 mismatched L4 protocol

Add a tdc test that checks the act_pedit extended L4 header mode does not
edit a packet whose IPv4 protocol does not match the selected transport
header.

The test installs an ingress pedit rule that sets the UDP destination
port, then injects a TCP packet with dport 2222. The UDP and TCP
destination ports sit at the same L4 offset, so a buggy kernel rewrites
the TCP dport. A second flower filter matches TCP dport 2222 and drops
the packet through an indexed gact action; the test then verifies via
JSON that this action saw exactly one packet, i.e. the dport was left
untouched and still matched 2222.

Signed-off-by: Victor Nogueira <victor@mojatatu.com>
---
 .../tc-testing/tc-tests/actions/pedit.json    | 49 +++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/pedit.json b/tools/testing/selftests/tc-testing/tc-tests/actions/pedit.json
index 37c4103321749..d8b685cfc62de 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/pedit.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/pedit.json
@@ -1920,5 +1920,54 @@
         "teardown": [
             "$TC actions flush action pedit"
         ]
+    },
+    {
+        "id": "1a4f",
+        "name": "Pedit udp dport should not mangle TCP packet dport",
+        "category": [
+            "actions",
+            "pedit"
+        ],
+        "plugins": {
+            "requires": [
+                "nsPlugin",
+                "scapyPlugin"
+            ]
+        },
+        "setup": [
+            "$TC qdisc add dev $DEV1 clsact",
+            "$TC filter add dev $DEV1 ingress protocol ip pref 1 matchall action pedit ex munge udp dport set 18053 continue"
+        ],
+        "cmdUnderTest": "$TC filter add dev $DEV1 ingress protocol ip pref 2 flower ip_proto tcp dst_port 2222 action drop index 1",
+        "scapy": {
+            "iface": "$DEV0",
+            "count": 1,
+            "packet": "Ether()/IP(dst='10.10.10.1')/TCP(dport=2222)"
+        },
+        "expExitCode": "0",
+        "verifyCmd": "$TC -j -s actions get action gact index 1",
+        "matchJSON": [
+            {
+                "total acts": 0
+            },
+            {
+                "actions": [
+                    {
+                        "order": 1,
+                        "kind": "gact",
+                        "control_action": {
+                            "type": "drop"
+                        },
+                        "index": 1,
+                        "stats": {
+                            "packets": 1
+                        }
+                    }
+                ]
+            }
+        ],
+        "teardown": [
+            "$TC qdisc del dev $DEV1 clsact"
+        ]
     }
 ]
-- 
2.54.0

Re: [PATCH] net/sched: act_pedit: require matching IPv4 L4 protocol

[Samuel Moelius]

On Mon, Jun 8, 2026 at 10:41 AM Victor Nogueira <victor@mojatatu.com> wrote:
>
> On Mon, Jun 8, 2026 at 7:46 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> >
> > On Sun, Jun 7, 2026 at 10:33 AM Samuel Moelius
> > <sam.moelius@trailofbits.com> wrote:
> > >
> > > On Sat, Jun 6, 2026 at 5:29 PM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> > > >
> > > > On Fri, Jun 5, 2026 at 3:46 PM Samuel Moelius
> > > > <sam.moelius@trailofbits.com> wrote:
> > > > >
> > > > > The extended IPv4 L4 header mode in act_pedit can select TCP or UDP
> > > > > header fields without confirming that the IPv4 protocol field matches
> > > > > the selected transport header.
> > > > >
> > > > > That lets a rule written for TCP or UDP modify unrelated payload bytes
> > > > > in a packet carrying a different protocol.
> > > > >
> > > > > Verify the IPv4 protocol before applying TCP or UDP extended header
> > > > > edits.
> > > > >
> > > > > Assisted-by: Codex:gpt-5.5-cyber-preview
> > > > > Signed-off-by: Samuel Moelius <sam.moelius@trailofbits.com>
> > > > > ---
> > > > >  net/sched/act_pedit.c | 2 ++
> > > > >  1 file changed, 2 insertions(+)
> > > > >
> > > > > diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
> > > > > index bc20f08a2789..9a4590451f7e 100644
> > > > > --- a/net/sched/act_pedit.c
> > > > > +++ b/net/sched/act_pedit.c
> > > > > @@ -341,6 +341,8 @@ static int pedit_l4_skb_offset(struct sk_buff *skb, int *hoffset, const int head
> > > > >
> > > > >                 if (!iph)
> > > > >                         goto out;
> > > > > +               if (iph->ihl < 5 || iph->protocol != header_type)
> > > > > +                       goto out;
> > > >
> > > > From inspection the fix looks reasonable.
> > > > At first glance it seems that the only fix that resolves the issue you
> > > > are describing is to check the protocol header.
> > > > header length seems to be an extra thing. But let's do what the v6
> > > > side seems to and skip frags as well? Something maybe along the lines
> > > > of:
> > > >
> > > > if (iph->ihl < 5 || iph->protocol != header_type || (iph->frag_off &
> > > > htons(IP_OFFSET)))
> > > >         goto out;
> > >
> > > I will submit an updated patch.
> > >
> > > > Are you able to describe how someone would configure a tc rule that
> > > > will allow this to happen?
> > > > IOW, how did you verify the problem and then validate that your fix works?
> > >
> > > The bug was validated using a custom init script in QEMU. The specific
> > > line used was:
> > >
> > > /usr/sbin/tc filter add dev veth0 egress protocol ip pref 1 matchall
> > > action pedit ex munge udp dport set 18053
> > >
> > > An IPv4 TCP packet with destination port 2222 was sent through that
> > > path. With the unpatched kernel, the packet was received with TCP
> > > destination port 18053. With the patched kernel, the same packet was
> > > received unchanged.
> > >
> > > I can provide more details or artifacts if you would like.
> >
> > A tdc test case is better. Victor or I can create one for you if you
> > dont know how.
>
> It seems like v2 isn't applying so you'll have to send a new version.
> When you do, after waiting for the 24h period, can you resend with
> the attached patch (containing the tdc test)? Since this is a
> fix, in the new version, remember to target net and add a proper fixes
> tag.

I'm sorry, I'm not quite following.

I should send a multipart patch with the patch you shared as one of the parts?

Re: [PATCH] net/sched: act_pedit: require matching IPv4 L4 protocol

[Victor Nogueira]

On Tue, Jun 9, 2026 at 10:57 AM Samuel Moelius
<sam.moelius@trailofbits.com> wrote:
>
> On Mon, Jun 8, 2026 at 10:41 AM Victor Nogueira <victor@mojatatu.com> wrote:
> >
> > On Mon, Jun 8, 2026 at 7:46 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> > >
> > > On Sun, Jun 7, 2026 at 10:33 AM Samuel Moelius
> > > <sam.moelius@trailofbits.com> wrote:
> > > >
> > > > On Sat, Jun 6, 2026 at 5:29 PM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> > > > >
> > > > > On Fri, Jun 5, 2026 at 3:46 PM Samuel Moelius
> > > > > <sam.moelius@trailofbits.com> wrote:
> > > > > >
> > > > > > The extended IPv4 L4 header mode in act_pedit can select TCP or UDP
> > > > > > header fields without confirming that the IPv4 protocol field matches
> > > > > > the selected transport header.
> > > > > >
> > > > > > That lets a rule written for TCP or UDP modify unrelated payload bytes
> > > > > > in a packet carrying a different protocol.
> > > > > >
> > > > > > Verify the IPv4 protocol before applying TCP or UDP extended header
> > > > > > edits.
> > > > > >
> > > > > > Assisted-by: Codex:gpt-5.5-cyber-preview
> > > > > > Signed-off-by: Samuel Moelius <sam.moelius@trailofbits.com>
> > > > > > ---
> > > > > >  net/sched/act_pedit.c | 2 ++
> > > > > >  1 file changed, 2 insertions(+)
> > > > > >
> > > > > > diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
> > > > > > index bc20f08a2789..9a4590451f7e 100644
> > > > > > --- a/net/sched/act_pedit.c
> > > > > > +++ b/net/sched/act_pedit.c
> > > > > > @@ -341,6 +341,8 @@ static int pedit_l4_skb_offset(struct sk_buff *skb, int *hoffset, const int head
> > > > > >
> > > > > >                 if (!iph)
> > > > > >                         goto out;
> > > > > > +               if (iph->ihl < 5 || iph->protocol != header_type)
> > > > > > +                       goto out;
> > > > >
> > > > > From inspection the fix looks reasonable.
> > > > > At first glance it seems that the only fix that resolves the issue you
> > > > > are describing is to check the protocol header.
> > > > > header length seems to be an extra thing. But let's do what the v6
> > > > > side seems to and skip frags as well? Something maybe along the lines
> > > > > of:
> > > > >
> > > > > if (iph->ihl < 5 || iph->protocol != header_type || (iph->frag_off &
> > > > > htons(IP_OFFSET)))
> > > > >         goto out;
> > > >
> > > > I will submit an updated patch.
> > > >
> > > > > Are you able to describe how someone would configure a tc rule that
> > > > > will allow this to happen?
> > > > > IOW, how did you verify the problem and then validate that your fix works?
> > > >
> > > > The bug was validated using a custom init script in QEMU. The specific
> > > > line used was:
> > > >
> > > > /usr/sbin/tc filter add dev veth0 egress protocol ip pref 1 matchall
> > > > action pedit ex munge udp dport set 18053
> > > >
> > > > An IPv4 TCP packet with destination port 2222 was sent through that
> > > > path. With the unpatched kernel, the packet was received with TCP
> > > > destination port 18053. With the patched kernel, the same packet was
> > > > received unchanged.
> > > >
> > > > I can provide more details or artifacts if you would like.
> > >
> > > A tdc test case is better. Victor or I can create one for you if you
> > > dont know how.
> >
> > It seems like v2 isn't applying so you'll have to send a new version.
> > When you do, after waiting for the 24h period, can you resend with
> > the attached patch (containing the tdc test)? Since this is a
> > fix, in the new version, remember to target net and add a proper fixes
> > tag.
>
> I'm sorry, I'm not quite following.
>
> I should send a multipart patch with the patch you shared as one of the parts?

Yes, please send a patchset with patch 1 being your fix and patch 2 the tdc
test.

cheers,
Victor