Re: [PATCH] netfilter: conntrack: clamp TCP window scale to TCP_MAX_WSCALE via ctnetlink

[Florian Westphal <fw@strlen.de>]

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