Re: [PATCH] netfilter: conntrack_amanda: fix port value truncation

[Chenguang Zhao]

在 2026/9/7 07:37, Aamir Ahmed 写道:
> amanda_help() uses simple_strtoul() to parse the port number from
> Amanda CONNECT replies, passing the result directly through htons()
> into a __be16.  simple_strtoul() returns unsigned long, so values
> above 65535 are silently truncated by the implicit conversion to u16
> inside htons().
>
> The existing check "port == 0 || len > 5" is insufficient: it
> catches values that truncate to zero (e.g. 65536) and strings longer
> than 5 digits, but misses values 65537-99999 whose u16 truncation is
> non-zero.  For example, port 65537 becomes port 1, creating a
> conntrack expectation for an unintended port.
>
> Parse into an unsigned long and explicitly reject values above 65535
> before the htons() conversion, mirroring the pattern used by the FTP
> helper's get_port() and the recent IPVS FTP fix (commit
> e625a9477d12).
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Aamir Ahmed <elb12345@xxxxxxxxxxxxx>
> ---
>  net/netfilter/nf_conntrack_amanda.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/net/netfilter/nf_conntrack_amanda.c b/net/netfilter/nf_conntrack_amanda.c
> index 14ae660491f3..057cef7e2a7e 100644
> --- a/net/netfilter/nf_conntrack_amanda.c
> +++ b/net/netfilter/nf_conntrack_amanda.c
> @@ -91,6 +91,7 @@ static int amanda_help(struct sk_buff *skb,
>  	char pbuf[sizeof("65535")], *tmp;
>  	u16 len;
>  	__be16 port;
> +	unsigned long tmp_port;
>  	int ret = NF_ACCEPT;
>  	nf_nat_amanda_hook_fn *nf_nat_amanda;
>  
> @@ -132,10 +133,11 @@ static int amanda_help(struct sk_buff *skb,
>  			break;
>  		pbuf[len] = '\0';
>  
> -		port = htons(simple_strtoul(pbuf, &tmp, 10));
> +		tmp_port = simple_strtoul(pbuf, &tmp, 10);
>  		len = tmp - pbuf;
> -		if (port == 0 || len > 5)
> +		if (tmp_port == 0 || tmp_port > 65535 || len > 5)
len = min_t(unsigned int, sizeof(pbuf) - 1, stop - off) already
limits the length so that it cannot be greater than 5. Could the
len > 5 check here be dropped?
>  			break;
> +		port = htons(tmp_port);
>  
>  		exp = nf_ct_expect_alloc(ct);
>  		if (exp == NULL) {

The subject is "[PATCH] ...". Since this is a bugfix, please use "[PATCH net] ...".

Reviewed-by: Chenguang Zhao <zhaochenguang@xxxxxxxxxx>

Thanks

Chenguang