* Re: [PATCH] netfilter: conntrack: clamp TCP window scale to TCP_MAX_WSCALE via ctnetlink
2026-03-24 16:30 [PATCH] netfilter: conntrack: clamp TCP window scale to TCP_MAX_WSCALE via ctnetlink David Carlier
@ 2026-03-24 16:37 ` Florian Westphal
2026-03-24 16:43 ` David CARLIER
2026-03-24 17:12 ` [PATCH] netfilter: ctnetlink: use netlink policy range checks David Carlier
1 sibling, 1 reply; 6+ messages in thread
From: Florian Westphal @ 2026-03-24 16:37 UTC (permalink / raw)
To: David Carlier; +Cc: Pablo Neira Ayuso, Phil Sutter, Eric Dumazet, netdev
David Carlier <devnexen@gmail.com> wrote:
> The normal TCP option parsing path clamps td_scale to TCP_MAX_WSCALE
> (14), but the ctnetlink path (nlattr_to_tcp) accepts any u8 value
> (0-255). This td_scale is then used as a shift count for u32 values,
> causing undefined behavior when >= 32.
>
> Use NLA_POLICY_MAX in the netlink policy to reject values exceeding
> TCP_MAX_WSCALE at parse time, consistent with how commit f900e1d77ee0
> ("netfilter: conntrack: add missing validation of SCTP netlink
> attributes") addressed similar missing validations.
>
> Fixes: c8e2078cfe41 ("[NETFILTER]: ctnetlink: add support for internal tcp connection tracking flags handling")
> Signed-off-by: David Carlier <devnexen@gmail.com>
> ---
> net/netfilter/nf_conntrack_proto_tcp.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c
> index 0c1d086e96cb..7d364337d71e 100644
> --- a/net/netfilter/nf_conntrack_proto_tcp.c
> +++ b/net/netfilter/nf_conntrack_proto_tcp.c
> @@ -1386,8 +1386,8 @@ static int tcp_to_nlattr(struct sk_buff *skb, struct nlattr *nla,
>
> static const struct nla_policy tcp_nla_policy[CTA_PROTOINFO_TCP_MAX+1] = {
> [CTA_PROTOINFO_TCP_STATE] = { .type = NLA_U8 },
> - [CTA_PROTOINFO_TCP_WSCALE_ORIGINAL] = { .type = NLA_U8 },
> - [CTA_PROTOINFO_TCP_WSCALE_REPLY] = { .type = NLA_U8 },
> + [CTA_PROTOINFO_TCP_WSCALE_ORIGINAL] = NLA_POLICY_MAX(NLA_U8, TCP_MAX_WSCALE),
> + [CTA_PROTOINFO_TCP_WSCALE_REPLY] = NLA_POLICY_MAX(NLA_U8, TCP_MAX_WSCALE),
> [CTA_PROTOINFO_TCP_FLAGS_ORIGINAL] = { .len = sizeof(struct nf_ct_tcp_flags) },
> [CTA_PROTOINFO_TCP_FLAGS_REPLY] = { .len = sizeof(struct nf_ct_tcp_flags) },
Makes sense, but I would prefer to update everything in one go, as in
this pending patch:
https://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf-next.git/commit/?h=testing&id=3b77a876d9b980daf27f0fd70eaa7344db7c36a7
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] netfilter: ctnetlink: use netlink policy range checks
2026-03-24 16:30 [PATCH] netfilter: conntrack: clamp TCP window scale to TCP_MAX_WSCALE via ctnetlink David Carlier
2026-03-24 16:37 ` Florian Westphal
@ 2026-03-24 17:12 ` David Carlier
2026-03-26 15:30 ` patchwork-bot+netdevbpf
1 sibling, 1 reply; 6+ messages in thread
From: David Carlier @ 2026-03-24 17:12 UTC (permalink / raw)
To: Pablo Neira Ayuso, Florian Westphal, Phil Sutter, Eric Dumazet
Cc: netdev, David Carlier
Replace manual range and mask validations with netlink policy
annotations in ctnetlink code paths, so that the netlink core rejects
invalid values early and can generate extack errors.
- CTA_PROTOINFO_TCP_STATE: reject values > TCP_CONNTRACK_SYN_SENT2 at
policy level, removing the manual >= TCP_CONNTRACK_MAX check.
- CTA_PROTOINFO_TCP_WSCALE_ORIGINAL/REPLY: reject values > TCP_MAX_WSCALE
(14). The normal TCP option parsing path already clamps to this value,
but the ctnetlink path accepted 0-255, causing undefined behavior when
used as a u32 shift count.
- CTA_FILTER_ORIG_FLAGS/REPLY_FLAGS: use NLA_POLICY_MASK with
CTA_FILTER_F_ALL, removing the manual mask checks.
- CTA_EXPECT_FLAGS: use NLA_POLICY_MASK with NF_CT_EXPECT_MASK, adding
a new mask define grouping all valid expect flags.
Extracted from a broader nf-next patch by Florian Westphal, scoped to
ctnetlink for the fixes tree.
Fixes: c8e2078cfe41 ("[NETFILTER]: ctnetlink: add support for internal tcp connection tracking flags handling")
Co-developed-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: David Carlier <devnexen@gmail.com>
---
.../uapi/linux/netfilter/nf_conntrack_common.h | 4 ++++
net/netfilter/nf_conntrack_netlink.c | 16 +++++-----------
net/netfilter/nf_conntrack_proto_tcp.c | 10 +++-------
3 files changed, 12 insertions(+), 18 deletions(-)
diff --git a/include/uapi/linux/netfilter/nf_conntrack_common.h b/include/uapi/linux/netfilter/nf_conntrack_common.h
index 26071021e986..56b6b60a814f 100644
--- a/include/uapi/linux/netfilter/nf_conntrack_common.h
+++ b/include/uapi/linux/netfilter/nf_conntrack_common.h
@@ -159,5 +159,9 @@ enum ip_conntrack_expect_events {
#define NF_CT_EXPECT_INACTIVE 0x2
#define NF_CT_EXPECT_USERSPACE 0x4
+#ifdef __KERNEL__
+#define NF_CT_EXPECT_MASK (NF_CT_EXPECT_PERMANENT | NF_CT_EXPECT_INACTIVE | \
+ NF_CT_EXPECT_USERSPACE)
+#endif
#endif /* _UAPI_NF_CONNTRACK_COMMON_H */
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index c156574e1273..d7a4207d112e 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -910,8 +910,8 @@ struct ctnetlink_filter {
};
static const struct nla_policy cta_filter_nla_policy[CTA_FILTER_MAX + 1] = {
- [CTA_FILTER_ORIG_FLAGS] = { .type = NLA_U32 },
- [CTA_FILTER_REPLY_FLAGS] = { .type = NLA_U32 },
+ [CTA_FILTER_ORIG_FLAGS] = NLA_POLICY_MASK(NLA_U32, CTA_FILTER_F_ALL),
+ [CTA_FILTER_REPLY_FLAGS] = NLA_POLICY_MASK(NLA_U32, CTA_FILTER_F_ALL),
};
static int ctnetlink_parse_filter(const struct nlattr *attr,
@@ -925,17 +925,11 @@ static int ctnetlink_parse_filter(const struct nlattr *attr,
if (ret)
return ret;
- if (tb[CTA_FILTER_ORIG_FLAGS]) {
+ if (tb[CTA_FILTER_ORIG_FLAGS])
filter->orig_flags = nla_get_u32(tb[CTA_FILTER_ORIG_FLAGS]);
- if (filter->orig_flags & ~CTA_FILTER_F_ALL)
- return -EOPNOTSUPP;
- }
- if (tb[CTA_FILTER_REPLY_FLAGS]) {
+ if (tb[CTA_FILTER_REPLY_FLAGS])
filter->reply_flags = nla_get_u32(tb[CTA_FILTER_REPLY_FLAGS]);
- if (filter->reply_flags & ~CTA_FILTER_F_ALL)
- return -EOPNOTSUPP;
- }
return 0;
}
@@ -2634,7 +2628,7 @@ static const struct nla_policy exp_nla_policy[CTA_EXPECT_MAX+1] = {
[CTA_EXPECT_HELP_NAME] = { .type = NLA_NUL_STRING,
.len = NF_CT_HELPER_NAME_LEN - 1 },
[CTA_EXPECT_ZONE] = { .type = NLA_U16 },
- [CTA_EXPECT_FLAGS] = { .type = NLA_U32 },
+ [CTA_EXPECT_FLAGS] = NLA_POLICY_MASK(NLA_BE32, NF_CT_EXPECT_MASK),
[CTA_EXPECT_CLASS] = { .type = NLA_U32 },
[CTA_EXPECT_NAT] = { .type = NLA_NESTED },
[CTA_EXPECT_FN] = { .type = NLA_NUL_STRING },
diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c
index 0c1d086e96cb..b67426c2189b 100644
--- a/net/netfilter/nf_conntrack_proto_tcp.c
+++ b/net/netfilter/nf_conntrack_proto_tcp.c
@@ -1385,9 +1385,9 @@ static int tcp_to_nlattr(struct sk_buff *skb, struct nlattr *nla,
}
static const struct nla_policy tcp_nla_policy[CTA_PROTOINFO_TCP_MAX+1] = {
- [CTA_PROTOINFO_TCP_STATE] = { .type = NLA_U8 },
- [CTA_PROTOINFO_TCP_WSCALE_ORIGINAL] = { .type = NLA_U8 },
- [CTA_PROTOINFO_TCP_WSCALE_REPLY] = { .type = NLA_U8 },
+ [CTA_PROTOINFO_TCP_STATE] = NLA_POLICY_MAX(NLA_U8, TCP_CONNTRACK_SYN_SENT2),
+ [CTA_PROTOINFO_TCP_WSCALE_ORIGINAL] = NLA_POLICY_MAX(NLA_U8, TCP_MAX_WSCALE),
+ [CTA_PROTOINFO_TCP_WSCALE_REPLY] = NLA_POLICY_MAX(NLA_U8, TCP_MAX_WSCALE),
[CTA_PROTOINFO_TCP_FLAGS_ORIGINAL] = { .len = sizeof(struct nf_ct_tcp_flags) },
[CTA_PROTOINFO_TCP_FLAGS_REPLY] = { .len = sizeof(struct nf_ct_tcp_flags) },
};
@@ -1414,10 +1414,6 @@ static int nlattr_to_tcp(struct nlattr *cda[], struct nf_conn *ct)
if (err < 0)
return err;
- if (tb[CTA_PROTOINFO_TCP_STATE] &&
- nla_get_u8(tb[CTA_PROTOINFO_TCP_STATE]) >= TCP_CONNTRACK_MAX)
- return -EINVAL;
-
spin_lock_bh(&ct->lock);
if (tb[CTA_PROTOINFO_TCP_STATE])
ct->proto.tcp.state = nla_get_u8(tb[CTA_PROTOINFO_TCP_STATE]);
--
2.53.0
^ permalink raw reply related [flat|nested] 6+ messages in thread