Re: [PATCH v2] net/sched: act_nat: only rewrite IPv4 packets

[Samuel Moelius]

On Tue, Jun 9, 2026 at 9:51 PM Jakub Kicinski <kuba@xxxxxxxxxx> wrote:
>
> 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_nat: only rewrite IPv4 packets
>
> This commit restricts act_nat to only process IPv4 packets to prevent
> non-IPv4 payloads from being incorrectly parsed and modified as IPv4
> headers. It adds a protocol check using skb_protocol() to skip inline
> VLAN tags.
>
> > diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c
> > index abb332dee836d..cb3e7a415da74 100644
> > --- a/net/sched/act_nat.c
> > +++ b/net/sched/act_nat.c
> > @@ -142,6 +142,9 @@ TC_INDIRECT_SCOPE int tcf_nat_act(struct sk_buff *skb,
> >       egress = parms->flags & TCA_NAT_FLAG_EGRESS;
> >
> >       noff = skb_network_offset(skb);
> > +     if (skb_protocol(skb, false) != htons(ETH_P_IP))
> > +             goto out;
> > +
>
> [Severity: High]
> Will this check skip valid IPv4 packets that have hardware-accelerated VLAN
> tags?
>
> When a packet has a hardware-accelerated VLAN tag, skb_vlan_tag_present(skb)
> is true. In this case, skb_protocol(skb, false) evaluates to skb->vlan_proto
> (such as ETH_P_8021Q), causing the check to evaluate to true and jump to the
> out label.
>
> Since hardware-accelerated VLAN tags are stripped by the MAC driver,
> skb_network_offset(skb) correctly points to the inner IPv4 header and
> skb->protocol already holds htons(ETH_P_IP). Should this code check
> skb->protocol != htons(ETH_P_IP) directly instead to avoid skipping
> hardware-VLAN-tagged IPv4 packets?
>
> >       if (!pskb_may_pull(skb, sizeof(*iph) + noff))
> >               goto drop;
> >
> --
> pw-bot: cr

I am getting ready to travel and I will address this when I return in
about two weeks. Thank you for understanding.