Re: [PATCH nf-next] netfilter: nft_payload: extend offset to 65535 bytes

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Thu, Aug 28, 2025 at 03:06:10PM +0200, Fernando Fernandez Mancera wrote:
> On 8/28/25 2:58 PM, Pablo Neira Ayuso wrote:
> > On Thu, Aug 28, 2025 at 02:48:31PM +0200, Fernando Fernandez Mancera wrote:
[...]
> > > diff --git a/net/netfilter/nft_payload.c b/net/netfilter/nft_payload.c
> > > index 7dfc5343dae4..728a4c78775c 100644
> > > --- a/net/netfilter/nft_payload.c
> > > +++ b/net/netfilter/nft_payload.c
> > > @@ -40,7 +40,7 @@ static bool nft_payload_rebuild_vlan_hdr(const struct sk_buff *skb, int mac_off,
> > >   /* add vlan header into the user buffer for if tag was removed by offloads */
> > >   static bool
> > > -nft_payload_copy_vlan(u32 *d, const struct sk_buff *skb, u8 offset, u8 len)
> > > +nft_payload_copy_vlan(u32 *d, const struct sk_buff *skb, u16 offset, u8 len)
> > >   {
> > >   	int mac_off = skb_mac_header(skb) - skb->data;
> > >   	u8 *vlanh, *dst_u8 = (u8 *) d;
> > > @@ -212,7 +212,7 @@ static const struct nla_policy nft_payload_policy[NFTA_PAYLOAD_MAX + 1] = {
> > >   	[NFTA_PAYLOAD_SREG]		= { .type = NLA_U32 },
> > >   	[NFTA_PAYLOAD_DREG]		= { .type = NLA_U32 },
> > >   	[NFTA_PAYLOAD_BASE]		= { .type = NLA_U32 },
> > > -	[NFTA_PAYLOAD_OFFSET]		= NLA_POLICY_MAX(NLA_BE32, 255),
> > > +	[NFTA_PAYLOAD_OFFSET]		= { .type = NLA_BE32 },
> > 
> > Should this be
> > 
> >                                          NLA_POLICY_MAX(NLA_BE32, 65535),
> > 
> > ?
> > 
> 
> Hi Pablo,
> 
> I don't think so. NLA_POLICY_MAX sets the nla_policy field "max" which is a
> 16 bit signed int (s16). Therefore, when doing NLA_POLICY_MAX(NLA_BE32,
> 65535) it triggers a warning as the max value set is actually "-1" in a s16.
> 
> This is why I decided to drop it. Let me know if I am missing something
> here..

Ah indeed, I forgot this NLA_POLICY_MAX limitation.

Thanks for explaining.