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

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

Posted by Samuel Moelius 2 days, 9 hours ago

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

Posted by Jamal Hadi Salim 1 day, 8 hours ago

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

Posted by Samuel Moelius 14 hours ago

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.