* [PATCH nf-next] netfilter: nft_payload: extend offset to 65535 bytes
@ 2025-08-28 12:48 Fernando Fernandez Mancera
2025-08-28 12:58 ` Pablo Neira Ayuso
0 siblings, 1 reply; 4+ messages in thread
From: Fernando Fernandez Mancera @ 2025-08-28 12:48 UTC (permalink / raw)
To: netfilter-devel; +Cc: coreteam, pablo, Fernando Fernandez Mancera
In some situations 255 bytes offset is not enough to match or manipulate
the desired packet field. Increase the offset limit to 65535 or U16_MAX.
In addition, the nla policy maximum value is not set anymore as it is
limited to s16. Instead, the maximum value is checked during the payload
expression initialization function.
Tested with the nft command line tool.
table ip filter {
chain output {
@nh,2040,8 set 0xff
@nh,524280,8 set 0xff
@nh,524280,8 0xff
@nh,2040,8 0xff
}
}
Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
---
include/net/netfilter/nf_tables_core.h | 2 +-
net/netfilter/nft_payload.c | 18 +++++++++++-------
2 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/include/net/netfilter/nf_tables_core.h b/include/net/netfilter/nf_tables_core.h
index 6c2f483d9828..7644cfe9267d 100644
--- a/include/net/netfilter/nf_tables_core.h
+++ b/include/net/netfilter/nf_tables_core.h
@@ -73,7 +73,7 @@ struct nft_ct {
struct nft_payload {
enum nft_payload_bases base:8;
- u8 offset;
+ u16 offset;
u8 len;
u8 dreg;
};
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 },
[NFTA_PAYLOAD_LEN] = NLA_POLICY_MAX(NLA_BE32, 255),
[NFTA_PAYLOAD_CSUM_TYPE] = { .type = NLA_U32 },
[NFTA_PAYLOAD_CSUM_OFFSET] = NLA_POLICY_MAX(NLA_BE32, 255),
@@ -797,7 +797,7 @@ static int nft_payload_csum_inet(struct sk_buff *skb, const u32 *src,
struct nft_payload_set {
enum nft_payload_bases base:8;
- u8 offset;
+ u16 offset;
u8 len;
u8 sreg;
u8 csum_type;
@@ -812,7 +812,7 @@ struct nft_payload_vlan_hdr {
};
static bool
-nft_payload_set_vlan(const u32 *src, struct sk_buff *skb, u8 offset, u8 len,
+nft_payload_set_vlan(const u32 *src, struct sk_buff *skb, u16 offset, u8 len,
int *vlan_hlen)
{
struct nft_payload_vlan_hdr *vlanh;
@@ -940,14 +940,18 @@ static int nft_payload_set_init(const struct nft_ctx *ctx,
const struct nft_expr *expr,
const struct nlattr * const tb[])
{
+ u32 csum_offset, offset, csum_type = NFT_PAYLOAD_CSUM_NONE;
struct nft_payload_set *priv = nft_expr_priv(expr);
- u32 csum_offset, csum_type = NFT_PAYLOAD_CSUM_NONE;
int err;
priv->base = ntohl(nla_get_be32(tb[NFTA_PAYLOAD_BASE]));
- priv->offset = ntohl(nla_get_be32(tb[NFTA_PAYLOAD_OFFSET]));
priv->len = ntohl(nla_get_be32(tb[NFTA_PAYLOAD_LEN]));
+ err = nft_parse_u32_check(tb[NFTA_PAYLOAD_OFFSET], U16_MAX, &offset);
+ if (err < 0)
+ return err;
+ priv->offset = offset;
+
if (tb[NFTA_PAYLOAD_CSUM_TYPE])
csum_type = ntohl(nla_get_be32(tb[NFTA_PAYLOAD_CSUM_TYPE]));
if (tb[NFTA_PAYLOAD_CSUM_OFFSET]) {
@@ -1069,7 +1073,7 @@ nft_payload_select_ops(const struct nft_ctx *ctx,
if (tb[NFTA_PAYLOAD_DREG] == NULL)
return ERR_PTR(-EINVAL);
- err = nft_parse_u32_check(tb[NFTA_PAYLOAD_OFFSET], U8_MAX, &offset);
+ err = nft_parse_u32_check(tb[NFTA_PAYLOAD_OFFSET], U16_MAX, &offset);
if (err < 0)
return ERR_PTR(err);
--
2.51.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH nf-next] netfilter: nft_payload: extend offset to 65535 bytes
2025-08-28 12:48 [PATCH nf-next] netfilter: nft_payload: extend offset to 65535 bytes Fernando Fernandez Mancera
@ 2025-08-28 12:58 ` Pablo Neira Ayuso
2025-08-28 13:06 ` Fernando Fernandez Mancera
0 siblings, 1 reply; 4+ messages in thread
From: Pablo Neira Ayuso @ 2025-08-28 12:58 UTC (permalink / raw)
To: Fernando Fernandez Mancera; +Cc: netfilter-devel, coreteam
On Thu, Aug 28, 2025 at 02:48:31PM +0200, Fernando Fernandez Mancera wrote:
> In some situations 255 bytes offset is not enough to match or manipulate
> the desired packet field. Increase the offset limit to 65535 or U16_MAX.
>
> In addition, the nla policy maximum value is not set anymore as it is
> limited to s16. Instead, the maximum value is checked during the payload
> expression initialization function.
>
> Tested with the nft command line tool.
>
> table ip filter {
> chain output {
> @nh,2040,8 set 0xff
> @nh,524280,8 set 0xff
> @nh,524280,8 0xff
> @nh,2040,8 0xff
> }
> }
>
> Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
> ---
> include/net/netfilter/nf_tables_core.h | 2 +-
> net/netfilter/nft_payload.c | 18 +++++++++++-------
> 2 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/include/net/netfilter/nf_tables_core.h b/include/net/netfilter/nf_tables_core.h
> index 6c2f483d9828..7644cfe9267d 100644
> --- a/include/net/netfilter/nf_tables_core.h
> +++ b/include/net/netfilter/nf_tables_core.h
> @@ -73,7 +73,7 @@ struct nft_ct {
>
> struct nft_payload {
> enum nft_payload_bases base:8;
> - u8 offset;
> + u16 offset;
> u8 len;
> u8 dreg;
> };
> 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),
?
IIRC, NLA_POLICY_MAX(NLA_BE32, X) deprecates nft_parse_u32_check.
> [NFTA_PAYLOAD_LEN] = NLA_POLICY_MAX(NLA_BE32, 255),
> [NFTA_PAYLOAD_CSUM_TYPE] = { .type = NLA_U32 },
> [NFTA_PAYLOAD_CSUM_OFFSET] = NLA_POLICY_MAX(NLA_BE32, 255),
> @@ -797,7 +797,7 @@ static int nft_payload_csum_inet(struct sk_buff *skb, const u32 *src,
>
> struct nft_payload_set {
> enum nft_payload_bases base:8;
> - u8 offset;
> + u16 offset;
> u8 len;
> u8 sreg;
> u8 csum_type;
> @@ -812,7 +812,7 @@ struct nft_payload_vlan_hdr {
> };
>
> static bool
> -nft_payload_set_vlan(const u32 *src, struct sk_buff *skb, u8 offset, u8 len,
> +nft_payload_set_vlan(const u32 *src, struct sk_buff *skb, u16 offset, u8 len,
> int *vlan_hlen)
> {
> struct nft_payload_vlan_hdr *vlanh;
> @@ -940,14 +940,18 @@ static int nft_payload_set_init(const struct nft_ctx *ctx,
> const struct nft_expr *expr,
> const struct nlattr * const tb[])
> {
> + u32 csum_offset, offset, csum_type = NFT_PAYLOAD_CSUM_NONE;
> struct nft_payload_set *priv = nft_expr_priv(expr);
> - u32 csum_offset, csum_type = NFT_PAYLOAD_CSUM_NONE;
> int err;
>
> priv->base = ntohl(nla_get_be32(tb[NFTA_PAYLOAD_BASE]));
> - priv->offset = ntohl(nla_get_be32(tb[NFTA_PAYLOAD_OFFSET]));
> priv->len = ntohl(nla_get_be32(tb[NFTA_PAYLOAD_LEN]));
>
> + err = nft_parse_u32_check(tb[NFTA_PAYLOAD_OFFSET], U16_MAX, &offset);
> + if (err < 0)
> + return err;
> + priv->offset = offset;
Then, this nft_parse_u32_check() can go away.
IIRC, nft_parse_u32_check() was added before NLA_POLICY_MAX(NLA_BE32, X)
existed.
Can anyone please validate this claims?
> +
> if (tb[NFTA_PAYLOAD_CSUM_TYPE])
> csum_type = ntohl(nla_get_be32(tb[NFTA_PAYLOAD_CSUM_TYPE]));
> if (tb[NFTA_PAYLOAD_CSUM_OFFSET]) {
> @@ -1069,7 +1073,7 @@ nft_payload_select_ops(const struct nft_ctx *ctx,
> if (tb[NFTA_PAYLOAD_DREG] == NULL)
> return ERR_PTR(-EINVAL);
>
> - err = nft_parse_u32_check(tb[NFTA_PAYLOAD_OFFSET], U8_MAX, &offset);
> + err = nft_parse_u32_check(tb[NFTA_PAYLOAD_OFFSET], U16_MAX, &offset);
> if (err < 0)
> return ERR_PTR(err);
>
> --
> 2.51.0
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH nf-next] netfilter: nft_payload: extend offset to 65535 bytes
2025-08-28 12:58 ` Pablo Neira Ayuso
@ 2025-08-28 13:06 ` Fernando Fernandez Mancera
2025-08-28 13:32 ` Pablo Neira Ayuso
0 siblings, 1 reply; 4+ messages in thread
From: Fernando Fernandez Mancera @ 2025-08-28 13:06 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel, coreteam
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:
>> In some situations 255 bytes offset is not enough to match or manipulate
>> the desired packet field. Increase the offset limit to 65535 or U16_MAX.
>>
>> In addition, the nla policy maximum value is not set anymore as it is
>> limited to s16. Instead, the maximum value is checked during the payload
>> expression initialization function.
>>
>> Tested with the nft command line tool.
>>
>> table ip filter {
>> chain output {
>> @nh,2040,8 set 0xff
>> @nh,524280,8 set 0xff
>> @nh,524280,8 0xff
>> @nh,2040,8 0xff
>> }
>> }
>>
>> Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
>> ---
>> include/net/netfilter/nf_tables_core.h | 2 +-
>> net/netfilter/nft_payload.c | 18 +++++++++++-------
>> 2 files changed, 12 insertions(+), 8 deletions(-)
>>
>> diff --git a/include/net/netfilter/nf_tables_core.h b/include/net/netfilter/nf_tables_core.h
>> index 6c2f483d9828..7644cfe9267d 100644
>> --- a/include/net/netfilter/nf_tables_core.h
>> +++ b/include/net/netfilter/nf_tables_core.h
>> @@ -73,7 +73,7 @@ struct nft_ct {
>>
>> struct nft_payload {
>> enum nft_payload_bases base:8;
>> - u8 offset;
>> + u16 offset;
>> u8 len;
>> u8 dreg;
>> };
>> 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..
> IIRC, NLA_POLICY_MAX(NLA_BE32, X) deprecates nft_parse_u32_check.
>
>> [NFTA_PAYLOAD_LEN] = NLA_POLICY_MAX(NLA_BE32, 255),
>> [NFTA_PAYLOAD_CSUM_TYPE] = { .type = NLA_U32 },
>> [NFTA_PAYLOAD_CSUM_OFFSET] = NLA_POLICY_MAX(NLA_BE32, 255),
>> @@ -797,7 +797,7 @@ static int nft_payload_csum_inet(struct sk_buff *skb, const u32 *src,
>>
>> struct nft_payload_set {
>> enum nft_payload_bases base:8;
>> - u8 offset;
>> + u16 offset;
>> u8 len;
>> u8 sreg;
>> u8 csum_type;
>> @@ -812,7 +812,7 @@ struct nft_payload_vlan_hdr {
>> };
>>
>> static bool
>> -nft_payload_set_vlan(const u32 *src, struct sk_buff *skb, u8 offset, u8 len,
>> +nft_payload_set_vlan(const u32 *src, struct sk_buff *skb, u16 offset, u8 len,
>> int *vlan_hlen)
>> {
>> struct nft_payload_vlan_hdr *vlanh;
>> @@ -940,14 +940,18 @@ static int nft_payload_set_init(const struct nft_ctx *ctx,
>> const struct nft_expr *expr,
>> const struct nlattr * const tb[])
>> {
>> + u32 csum_offset, offset, csum_type = NFT_PAYLOAD_CSUM_NONE;
>> struct nft_payload_set *priv = nft_expr_priv(expr);
>> - u32 csum_offset, csum_type = NFT_PAYLOAD_CSUM_NONE;
>> int err;
>>
>> priv->base = ntohl(nla_get_be32(tb[NFTA_PAYLOAD_BASE]));
>> - priv->offset = ntohl(nla_get_be32(tb[NFTA_PAYLOAD_OFFSET]));
>> priv->len = ntohl(nla_get_be32(tb[NFTA_PAYLOAD_LEN]));
>>
>> + err = nft_parse_u32_check(tb[NFTA_PAYLOAD_OFFSET], U16_MAX, &offset);
>> + if (err < 0)
>> + return err;
>> + priv->offset = offset;
>
> Then, this nft_parse_u32_check() can go away.
>
> IIRC, nft_parse_u32_check() was added before NLA_POLICY_MAX(NLA_BE32, X)
> existed.
>
> Can anyone please validate this claims?
>
>> +
>> if (tb[NFTA_PAYLOAD_CSUM_TYPE])
>> csum_type = ntohl(nla_get_be32(tb[NFTA_PAYLOAD_CSUM_TYPE]));
>> if (tb[NFTA_PAYLOAD_CSUM_OFFSET]) {
>> @@ -1069,7 +1073,7 @@ nft_payload_select_ops(const struct nft_ctx *ctx,
>> if (tb[NFTA_PAYLOAD_DREG] == NULL)
>> return ERR_PTR(-EINVAL);
>>
>> - err = nft_parse_u32_check(tb[NFTA_PAYLOAD_OFFSET], U8_MAX, &offset);
>> + err = nft_parse_u32_check(tb[NFTA_PAYLOAD_OFFSET], U16_MAX, &offset);
>> if (err < 0)
>> return ERR_PTR(err);
>>
>> --
>> 2.51.0
>>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH nf-next] netfilter: nft_payload: extend offset to 65535 bytes
2025-08-28 13:06 ` Fernando Fernandez Mancera
@ 2025-08-28 13:32 ` Pablo Neira Ayuso
0 siblings, 0 replies; 4+ messages in thread
From: Pablo Neira Ayuso @ 2025-08-28 13:32 UTC (permalink / raw)
To: Fernando Fernandez Mancera; +Cc: netfilter-devel, coreteam
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-08-28 13:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-28 12:48 [PATCH nf-next] netfilter: nft_payload: extend offset to 65535 bytes Fernando Fernandez Mancera
2025-08-28 12:58 ` Pablo Neira Ayuso
2025-08-28 13:06 ` Fernando Fernandez Mancera
2025-08-28 13:32 ` Pablo Neira Ayuso
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.