[PATCH nf-next v4 0/4] netfilter: conntrack: shared port parser for helpers

[PATCH nf-next v4 0/4] netfilter: conntrack: shared port parser for helpers

[Rahul Chandelkar]

Both nf_conntrack_irc and nf_conntrack_amanda parsed port numbers from
application-layer payload with simple_strtoul(), which requires a
NUL-terminated string and returns unsigned long without any range check,
so a value above 65535 was silently truncated when stored in a u16 (e.g.
65537 became 1) and a conntrack expectation was created for a port that
never appeared in the payload.

Instead of open-coding a range check in each helper, this series adds a
pair of length-delimited parsers to the conntrack helper core and uses
them from the IRC, Amanda and SIP helpers:

  nf_ct_helper_parse_uint() - bounded decimal parser for an unterminated
    buffer, capped at UINT_MAX.  Its body is taken from
    nf_conntrack_sip's sip_strtouint().

  nf_ct_helper_parse_port() - wrapper that rejects port 0 and values
    above 65535.

Patch 1 adds the two helpers.  Patches 2 and 3 convert the IRC and Amanda
helpers (the actual truncation fixes).  Patch 4 removes the now-duplicate
sip_strtouint() and the open-coded digit loop in sip_parse_port() from
nf_conntrack_sip and uses the shared parsers, so there is a single
implementation in the tree.

Compile-tested (W=1) with NF_CONNTRACK_{IRC,AMANDA,SIP,FTP}=m.

v4:
  - Rebased onto nf-next.
  - Move sip_strtouint() into the helper core as
    nf_ct_helper_parse_uint() and build nf_ct_helper_parse_port() on top
    of it, rather than adding a second uint parser; convert
    nf_conntrack_sip to the shared helpers and drop its private copies
    (Pablo Neira Ayuso, Phil Sutter, Florian Westphal).
  - Target nf-next (Pablo Neira Ayuso, Jakub Kicinski).
v3: https://lore.kernel.org/all/20260503083220.630655-1-rc@rexion.ai/
  - Added nf_ct_helper_parse_uint() as the generic base and a fourth
    patch converting nf_conntrack_sip (Phil Sutter).
v2: https://lore.kernel.org/all/20260501063156.2520780-1-rc@rexion.ai/
  - Introduced a shared nf_ct_helper_parse_port() in the helper core
    instead of open-coding range checks in each helper (Florian Westphal,
    Pablo Neira Ayuso).
v1: https://lore.kernel.org/all/20260430161230.3438973-1-rc@rexion.ai/

Rahul Chandelkar (4):
  netfilter: conntrack: add shared uint and port parsers for helpers
  netfilter: nf_conntrack_irc: use nf_ct_helper_parse_port()
  netfilter: nf_conntrack_amanda: use nf_ct_helper_parse_port()
  netfilter: nf_conntrack_sip: use shared helper parsers

 include/net/netfilter/nf_conntrack_helper.h |  5 ++
 net/netfilter/nf_conntrack_amanda.c         |  8 +-
 net/netfilter/nf_conntrack_helper.c         | 68 ++++++++++++++++
 net/netfilter/nf_conntrack_irc.c            |  4 +-
 net/netfilter/nf_conntrack_sip.c            | 86 +++++----------------
 5 files changed, 98 insertions(+), 73 deletions(-)


base-commit: 87b80c2f6b05cad9f0ff9136709c62a0f59923e3
-- 
2.43.0

[PATCH nf-next v4 1/4] netfilter: conntrack: add shared uint and port parsers for helpers

[Rahul Chandelkar]

Several conntrack helpers parse integers and port numbers out of
application-layer payload.  Some open-code simple_strtoul() (which
requires a NUL-terminated string and returns unsigned long without any
range check), while nf_conntrack_sip carries its own sip_strtouint().

Add two length-delimited parsers to the conntrack helper core so helpers
can share one implementation:

  nf_ct_helper_parse_uint() - bounded decimal parser that operates on an
    unterminated buffer, capping the result at UINT_MAX.  Its body is
    taken from sip_strtouint() so nf_conntrack_sip can drop its private
    copy in a later patch.

  nf_ct_helper_parse_port() - thin wrapper that parses a port and rejects
    zero and values above 65535.

Both are exported so the IRC, Amanda and SIP helpers can use them.

Signed-off-by: Rahul Chandelkar <rc@rexion.ai>
---
 include/net/netfilter/nf_conntrack_helper.h |  5 ++
 net/netfilter/nf_conntrack_helper.c         | 68 +++++++++++++++++++++
 2 files changed, 73 insertions(+)

diff --git a/include/net/netfilter/nf_conntrack_helper.h b/include/net/netfilter/nf_conntrack_helper.h
index 335b8c43694f..4d5ad5ae1d13 100644
--- a/include/net/netfilter/nf_conntrack_helper.h
+++ b/include/net/netfilter/nf_conntrack_helper.h
@@ -184,6 +184,11 @@ nf_ct_helper_expectfn_find_by_name(const char *name);
 struct nf_ct_helper_expectfn *
 nf_ct_helper_expectfn_find_by_symbol(const void *symbol);
 
+unsigned int nf_ct_helper_parse_uint(const char *cp, unsigned int len,
+				     char **endp);
+int nf_ct_helper_parse_port(const char *cp, unsigned int len,
+			    u16 *port, char **endp);
+
 extern struct hlist_head *nf_ct_helper_hash;
 extern unsigned int nf_ct_helper_hsize;
 
diff --git a/net/netfilter/nf_conntrack_helper.c b/net/netfilter/nf_conntrack_helper.c
index c30ae3f203be..99b90a54d8a9 100644
--- a/net/netfilter/nf_conntrack_helper.c
+++ b/net/netfilter/nf_conntrack_helper.c
@@ -16,6 +16,7 @@
 #include <linux/random.h>
 #include <linux/err.h>
 #include <linux/kernel.h>
+#include <linux/ctype.h>
 #include <linux/netdevice.h>
 #include <linux/rculist.h>
 #include <linux/rtnetlink.h>
@@ -569,6 +570,73 @@ void nf_nat_helper_unregister(struct nf_conntrack_nat_helper *nat)
 }
 EXPORT_SYMBOL_GPL(nf_nat_helper_unregister);
 
+/* Parse a decimal unsigned integer from a length-delimited buffer that is not
+ * necessarily NUL-terminated.  At most @len bytes are examined.  On success
+ * the parsed value is returned and, if @endp is non-NULL, *@endp points just
+ * past the last digit consumed.  If no digit is found or the value would
+ * exceed UINT_MAX, 0 is returned and *@endp is set to @cp.
+ */
+unsigned int nf_ct_helper_parse_uint(const char *cp, unsigned int len,
+				     char **endp)
+{
+	const unsigned int max = sizeof("4294967295");
+	unsigned int olen = len;
+	const char *s = cp;
+	u64 result = 0;
+
+	if (len > max)
+		len = max;
+
+	while (olen > 0 && isdigit(*s)) {
+		unsigned int value;
+
+		if (len == 0)
+			goto err;
+
+		value = *s - '0';
+		result = result * 10 + value;
+
+		if (result > UINT_MAX)
+			goto err;
+		s++;
+		len--;
+		olen--;
+	}
+
+	if (endp)
+		*endp = (char *)s;
+
+	return result;
+err:
+	if (endp)
+		*endp = (char *)cp;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(nf_ct_helper_parse_uint);
+
+/* Parse a TCP/UDP port (1-65535) from a length-delimited buffer using
+ * nf_ct_helper_parse_uint().  Returns 0 and stores the port in *@port (and the
+ * end pointer in *@endp) on success, or -1 if no digit is found or the value
+ * is out of range.
+ */
+int nf_ct_helper_parse_port(const char *cp, unsigned int len,
+			    u16 *port, char **endp)
+{
+	char *e = (char *)cp;
+	unsigned int val;
+
+	val = nf_ct_helper_parse_uint(cp, len, &e);
+	if (e == cp || val == 0 || val > 65535)
+		return -1;
+
+	*port = val;
+	if (endp)
+		*endp = e;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(nf_ct_helper_parse_port);
+
 int nf_conntrack_helper_init(void)
 {
 	nf_ct_helper_hsize = 1; /* gets rounded up to use one page */
-- 
2.43.0

[PATCH nf-next v4 2/4] netfilter: nf_conntrack_irc: use nf_ct_helper_parse_port()

[Rahul Chandelkar]

parse_dcc() stored the result of simple_strtoul() directly into a u16
*port, so a DCC port above 65535 was silently truncated (e.g. 65537
became 1), creating a conntrack expectation for a port that never
appeared in the DCC command.

Use nf_ct_helper_parse_port(), which parses the length-delimited buffer
without relying on NUL termination and rejects port 0 and values above
65535.

Fixes: 869f37d8e48f ("[NETFILTER]: nf_conntrack/nf_nat: add IRC helper port")
Signed-off-by: Rahul Chandelkar <rc@rexion.ai>
---
 net/netfilter/nf_conntrack_irc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nf_conntrack_irc.c b/net/netfilter/nf_conntrack_irc.c
index 92360963757a..268ce7c1f9b2 100644
--- a/net/netfilter/nf_conntrack_irc.c
+++ b/net/netfilter/nf_conntrack_irc.c
@@ -88,7 +88,9 @@ static int parse_dcc(char *data, const char *data_end, __be32 *ip,
 		data++;
 	}
 
-	*port = simple_strtoul(data, &data, 10);
+	if (nf_ct_helper_parse_port(data, data_end - data, port, &data))
+		return -1;
+
 	*ad_end_p = data;
 
 	return 0;
-- 
2.43.0

[PATCH nf-next v4 3/4] netfilter: nf_conntrack_amanda: use nf_ct_helper_parse_port()

[Rahul Chandelkar]

amanda_help() passed the result of simple_strtoul() straight through
htons() into a __be16.  The digit string is copied into a 6-byte buffer,
so at most five digits are parsed, but values 65536-99999 still truncated
on the u16 conversion (e.g. 65537 became 1).

Use nf_ct_helper_parse_port(), which rejects port 0 and values above
65535 in one call.  The trailing len is still derived from the parser's
end pointer for the NAT mangle path.

Fixes: 16958900578b ("[NETFILTER]: nf_conntrack/nf_nat: add amanda helper port")
Signed-off-by: Rahul Chandelkar <rc@rexion.ai>
---
 net/netfilter/nf_conntrack_amanda.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/netfilter/nf_conntrack_amanda.c b/net/netfilter/nf_conntrack_amanda.c
index 14ae660491f3..f157c96b7d22 100644
--- a/net/netfilter/nf_conntrack_amanda.c
+++ b/net/netfilter/nf_conntrack_amanda.c
@@ -89,7 +89,7 @@ static int amanda_help(struct sk_buff *skb,
 	struct nf_conntrack_tuple *tuple;
 	unsigned int dataoff, start, stop, off, i;
 	char pbuf[sizeof("65535")], *tmp;
-	u16 len;
+	u16 len, parsed_port;
 	__be16 port;
 	int ret = NF_ACCEPT;
 	nf_nat_amanda_hook_fn *nf_nat_amanda;
@@ -132,10 +132,10 @@ static int amanda_help(struct sk_buff *skb,
 			break;
 		pbuf[len] = '\0';
 
-		port = htons(simple_strtoul(pbuf, &tmp, 10));
-		len = tmp - pbuf;
-		if (port == 0 || len > 5)
+		if (nf_ct_helper_parse_port(pbuf, len, &parsed_port, &tmp))
 			break;
+		port = htons(parsed_port);
+		len = tmp - pbuf;
 
 		exp = nf_ct_expect_alloc(ct);
 		if (exp == NULL) {
-- 
2.43.0

[PATCH nf-next v4 4/4] netfilter: nf_conntrack_sip: use shared helper parsers

[Rahul Chandelkar]

nf_conntrack_sip open-coded its own bounded integer parser
(sip_strtouint()) and its own digit loop in sip_parse_port().  Now that
equivalent parsers live in the conntrack helper core, drop the private
copies:

  - sip_strtouint() is replaced by nf_ct_helper_parse_uint() (same
    signature and semantics) at all call sites and removed.

  - sip_parse_port() uses nf_ct_helper_parse_port() for the numeric part,
    keeping the SIP-specific handling of the ':' separator, the default
    SIP_PORT and the minimum port of 1024.

No functional change intended.

Signed-off-by: Rahul Chandelkar <rc@rexion.ai>
---
 net/netfilter/nf_conntrack_sip.c | 86 +++++++-------------------------
 1 file changed, 18 insertions(+), 68 deletions(-)

diff --git a/net/netfilter/nf_conntrack_sip.c b/net/netfilter/nf_conntrack_sip.c
index 64bc440b1181..0e12426227a8 100644
--- a/net/netfilter/nf_conntrack_sip.c
+++ b/net/netfilter/nf_conntrack_sip.c
@@ -183,8 +183,8 @@ static int sip_parse_addr(const struct nf_conn *ct, const char *cp,
 static bool sip_parse_port(const char *dptr, const char **endp,
 			   const char *limit, __be16 *port)
 {
-	unsigned int p = 0;
-	int len = 0;
+	char *end;
+	u16 p;
 
 	if (dptr >= limit)
 		return false;
@@ -199,29 +199,22 @@ static bool sip_parse_port(const char *dptr, const char **endp,
 
 	dptr++; /* skip ':' */
 
-	while (dptr < limit && isdigit(*dptr)) {
-		p = p * 10 + (*dptr - '0');
-		dptr++;
-		len++;
-		if (len > 5) /* max "65535" */
-			return false;
-	}
-
-	if (len == 0)
+	if (nf_ct_helper_parse_port(dptr, limit - dptr, &p, &end))
 		return false;
 
 	/* reached limit while parsing port */
-	if (dptr >= limit)
+	if (end >= limit)
 		return false;
 
-	if (p < 1024 || p > 65535)
+	/* SIP ports below 1024 are not accepted */
+	if (p < 1024)
 		return false;
 
 	if (port)
 		*port = htons(p);
 
 	if (endp)
-		*endp = dptr;
+		*endp = end;
 
 	return true;
 }
@@ -270,51 +263,6 @@ static int skp_epaddr_len(const struct nf_conn *ct, const char *dptr,
 	return epaddr_len(ct, dptr, limit, shift);
 }
 
-/* simple_strtoul stops after first non-number character.
- * But as we're not dealing with c-strings, we can't rely on
- * hitting \r,\n,\0 etc. before moving past end of buffer.
- *
- * This is a variant of simple_strtoul, but doesn't require
- * a c-string.
- *
- * If value exceeds UINT_MAX, 0 is returned.
- */
-static unsigned int sip_strtouint(const char *cp, unsigned int len, char **endp)
-{
-	const unsigned int max = sizeof("4294967295");
-	unsigned int olen = len;
-	const char *s = cp;
-	u64 result = 0;
-
-	if (len > max)
-		len = max;
-
-	while (olen > 0 && isdigit(*s)) {
-		unsigned int value;
-
-		if (len == 0)
-			goto err;
-
-		value = *s - '0';
-		result = result * 10 + value;
-
-		if (result > UINT_MAX)
-			goto err;
-		s++;
-		len--;
-		olen--;
-	}
-
-	if (endp)
-		*endp = (char *)s;
-
-	return result;
-err:
-	if (endp)
-		*endp = (char *)cp;
-	return 0;
-}
-
 /* Parse a SIP request line of the form:
  *
  * Request-Line = Method SP Request-URI SP SIP-Version CRLF
@@ -682,7 +630,7 @@ int ct_sip_parse_numerical_param(const struct nf_conn *ct, const char *dptr,
 		return 0;
 
 	start += strlen(name);
-	*val = sip_strtouint(start, limit - start, (char **)&end);
+	*val = nf_ct_helper_parse_uint(start, limit - start, (char **)&end);
 	if (start == end)
 		return -1;
 	if (matchoff && matchlen) {
@@ -1166,7 +1114,8 @@ static int process_sdp(struct sk_buff *skb, unsigned int protoff,
 		mediaoff += t->len;
 		medialen -= t->len;
 
-		port = sip_strtouint(*dptr + mediaoff, *datalen - mediaoff, (char **)&end);
+		port = nf_ct_helper_parse_uint(*dptr + mediaoff, *datalen - mediaoff,
+					       (char **)&end);
 		if (port == 0 || *dptr + mediaoff == end)
 			continue;
 		if (port < 1024 || port > 65535) {
@@ -1357,7 +1306,7 @@ static int process_register_request(struct sk_buff *skb, unsigned int protoff,
 	 */
 	if (ct_sip_get_header(ct, *dptr, 0, *datalen, SIP_HDR_EXPIRES,
 			      &matchoff, &matchlen) > 0)
-		expires = sip_strtouint(*dptr + matchoff, *datalen - matchoff, NULL);
+		expires = nf_ct_helper_parse_uint(*dptr + matchoff, *datalen - matchoff, NULL);
 
 	ret = ct_sip_parse_header_uri(ct, *dptr, NULL, *datalen,
 				      SIP_HDR_CONTACT, NULL,
@@ -1467,7 +1416,7 @@ static int process_register_response(struct sk_buff *skb, unsigned int protoff,
 
 	if (ct_sip_get_header(ct, *dptr, 0, *datalen, SIP_HDR_EXPIRES,
 			      &matchoff, &matchlen) > 0)
-		expires = sip_strtouint(*dptr + matchoff, *datalen - matchoff, NULL);
+		expires = nf_ct_helper_parse_uint(*dptr + matchoff, *datalen - matchoff, NULL);
 
 	while (1) {
 		unsigned int c_expires = expires;
@@ -1531,8 +1480,8 @@ static int process_sip_response(struct sk_buff *skb, unsigned int protoff,
 
 	if (*datalen < strlen("SIP/2.0 200"))
 		return NF_ACCEPT;
-	code = sip_strtouint(*dptr + strlen("SIP/2.0 "),
-			     *datalen - strlen("SIP/2.0 "), NULL);
+	code = nf_ct_helper_parse_uint(*dptr + strlen("SIP/2.0 "),
+				       *datalen - strlen("SIP/2.0 "), NULL);
 	if (!code) {
 		nf_ct_helper_log(skb, ct, "cannot get code");
 		return NF_DROP;
@@ -1543,7 +1492,7 @@ static int process_sip_response(struct sk_buff *skb, unsigned int protoff,
 		nf_ct_helper_log(skb, ct, "cannot parse cseq");
 		return NF_DROP;
 	}
-	cseq = sip_strtouint(*dptr + matchoff, *datalen - matchoff, (char **)&end);
+	cseq = nf_ct_helper_parse_uint(*dptr + matchoff, *datalen - matchoff, (char **)&end);
 	if (*dptr + matchoff == end) {
 		nf_ct_helper_log(skb, ct, "cannot get cseq");
 		return NF_DROP;
@@ -1613,7 +1562,8 @@ static int process_sip_request(struct sk_buff *skb, unsigned int protoff,
 			nf_ct_helper_log(skb, ct, "cannot parse cseq");
 			return NF_DROP;
 		}
-		cseq = sip_strtouint(*dptr + matchoff, *datalen - matchoff, (char **)&end);
+		cseq = nf_ct_helper_parse_uint(*dptr + matchoff, *datalen - matchoff,
+					       (char **)&end);
 		if (*dptr + matchoff == end) {
 			nf_ct_helper_log(skb, ct, "cannot get cseq");
 			return NF_DROP;
@@ -1690,7 +1640,7 @@ static int sip_help_tcp(struct sk_buff *skb, unsigned int protoff,
 				      &matchoff, &matchlen) <= 0)
 			break;
 
-		clen = sip_strtouint(dptr + matchoff, datalen - matchoff, (char **)&end);
+		clen = nf_ct_helper_parse_uint(dptr + matchoff, datalen - matchoff, (char **)&end);
 		if (dptr + matchoff == end)
 			break;
 
-- 
2.43.0

Re: [PATCH nf-next v4 1/4] netfilter: conntrack: add shared uint and port parsers for helpers

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] nf_ct_helper_parse_uint() rejects any run of 12 or more digits,
  even when the value fits in UINT_MAX, and neither comment mentions…

--

netfilter: conntrack: add shared uint and port parsers for helpers

This commit adds nf_ct_helper_parse_uint() and nf_ct_helper_parse_port()
to the conntrack helper core. Both parse decimal numbers from a buffer of
a given length. The uint parser body is taken from sip_strtouint(), and
both functions are exported for the IRC, Amanda and SIP helpers.

> diff --git a/net/netfilter/nf_conntrack_helper.c b/net/netfilter/nf_conntrack_helper.c
> index c30ae3f203bee..99b90a54d8a9f 100644
> --- a/net/netfilter/nf_conntrack_helper.c
> +++ b/net/netfilter/nf_conntrack_helper.c

[ ... ]

> @@ -569,6 +570,73 @@ void nf_nat_helper_unregister(struct nf_conntrack_nat_helper *nat)
>  }
>  EXPORT_SYMBOL_GPL(nf_nat_helper_unregister);
>  
> +/* Parse a decimal unsigned integer from a length-delimited buffer that is not
> + * necessarily NUL-terminated.  At most @len bytes are examined.  On success
> + * the parsed value is returned and, if @endp is non-NULL, *@endp points just
> + * past the last digit consumed.  If no digit is found or the value would
> + * exceed UINT_MAX, 0 is returned and *@endp is set to @cp.
> + */
> +unsigned int nf_ct_helper_parse_uint(const char *cp, unsigned int len,
> +				     char **endp)
> +{
> +	const unsigned int max = sizeof("4294967295");
> +	unsigned int olen = len;
> +	const char *s = cp;
> +	u64 result = 0;
> +
> +	if (len > max)
> +		len = max;
> +
> +	while (olen > 0 && isdigit(*s)) {
> +		unsigned int value;
> +
> +		if (len == 0)
> +			goto err;

[Severity: Low]
Should a run of 12 or more digits be rejected here when the value still
fits in UINT_MAX?

max is sizeof("4294967295"), which is 11 because sizeof counts the NUL.
For longer inputs, len is clamped to 11 while olen keeps the real length.

The loop condition only checks olen and isdigit(). So a 12th digit enters
the loop and finds len == 0. It then takes the err path, which returns 0
and sets *endp to cp.

With leading zeros, result stays small, so this failure comes only from
the digit count. The count limit does not seem to add overflow
protection. result is a u64 and is compared against UINT_MAX after every
digit.

The comment above lists only "no digit is found" and "exceed UINT_MAX" as
failure cases. The nf_ct_helper_parse_port() comment lists only "no digit
is found" and "out of range".

nf_ct_helper_parse_port() treats e == cp as a failure:

	val = nf_ct_helper_parse_uint(cp, len, &e);
	if (e == cp || val == 0 || val > 65535)
		return -1;

So an input like "000000000080" returns -1 there.

This logic came from sip_strtouint(), but it is now a general exported
API. Later in the series, "netfilter: nf_conntrack_irc: use
nf_ct_helper_parse_port()" replaces simple_strtoul() in parse_dcc().
simple_strtoul() accepted any number of leading zeros.

Would a DCC command such as "DCC SEND f <ip> 0000000000006667" then fail
to parse, so that no expectation is created for it?

Amanda does not look affected, because pbuf limits its input to 5
characters.

Could the digit-count limit be dropped, or else documented in both
comments?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260923091608.2617604-1-rc%40rexion.ai

Re: [PATCH nf-next v4 2/4] netfilter: nf_conntrack_irc: use nf_ct_helper_parse_port()

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] After this patch, the `|| dcc_port == 0` test in help()
  (net/netfilter/nf_conntrack_irc.c) can never be true.
- [Low] Switching parse_dcc() to nf_ct_helper_parse_port() adds a digit-
  count limit to DCC port parsing that the kernel-doc does not describe.

--

netfilter: nf_conntrack_irc: use nf_ct_helper_parse_port()

This commit replaces simple_strtoul() in parse_dcc() with
nf_ct_helper_parse_port(). DCC ports above 65535 are no longer silently
truncated into the u16 port. The new helper parses a length-delimited
buffer and rejects port 0 and values above 65535.

> diff --git a/net/netfilter/nf_conntrack_irc.c b/net/netfilter/nf_conntrack_irc.c
> index 92360963757a9..268ce7c1f9b23 100644
> --- a/net/netfilter/nf_conntrack_irc.c
> +++ b/net/netfilter/nf_conntrack_irc.c
> @@ -88,7 +88,9 @@ static int parse_dcc(char *data, const char *data_end, __be32 *ip,
>  		data++;
>  	}
>  
> -	*port = simple_strtoul(data, &data, 10);
> +	if (nf_ct_helper_parse_port(data, data_end - data, port, &data))
> +		return -1;

[Severity: Low]

nf_ct_helper_parse_port() now returns -1 when val == 0. Can the
dcc_port == 0 test in help() still be true after that?

net/netfilter/nf_conntrack_irc.c:help() {
    ...
			if ((tuple->src.u3.ip != dcc_ip &&
			     ct->tuplehash[!dir].tuple.dst.u3.ip != dcc_ip) ||
			    dcc_port == 0) {
				net_warn_ratelimited("Forged DCC command from %pI4: %pI4:%u\n",
    ...
}

Before this patch, each of these inputs left dcc_port at 0: a port of
"0", a non-numeric port, or a value that truncated to 0 such as 65536.
In those cases parse_dcc() succeeded, and help() printed the ratelimited
"Forged DCC command" warning before goto out.

Now those commands fail in parse_dcc(), and help() takes this path
instead:

			if (parse_dcc(data, data_limit, &dcc_ip,
				       &dcc_port, &addr_beg_p, &addr_end_p)) {
				pr_debug("unable to parse dcc command\n");
				goto out;
			}

The result is the same either way: no expectation is created and the
packet gets NF_ACCEPT. The only difference is that the warning is no
longer printed. The dcc_port == 0 clause is still in help() at the end
of the series.

Should that clause be removed, with the logging change mentioned in the
commit message? Reverse/passive DCC also uses port 0, so it may be fine
to drop the "Forged" warning.

[Severity: Low]

Does this also reject in-range ports written with leading zeros?
nf_ct_helper_parse_uint() clamps len to 11 and counts every digit it
consumes, including leading zeros:

net/netfilter/nf_conntrack_helper.c:nf_ct_helper_parse_uint() {
	const unsigned int max = sizeof("4294967295");
    ...
	if (len > max)
		len = max;

	while (olen > 0 && isdigit(*s)) {
		unsigned int value;

		if (len == 0)
			goto err;
    ...
}

A DCC port such as "000000000001" reaches goto err on the 12th digit,
so parse_dcc() returns -1. simple_strtoul() used to accept that string
as port 1. help() now does goto out without creating an expectation or
calling the NAT hook.

The kernel-doc for nf_ct_helper_parse_uint() lists only two failure
cases: "no digit is found or the value would exceed UINT_MAX". The
comment for nf_ct_helper_parse_port() says "no digit is found or the
value is out of range". Neither one mentions the digit limit.

This logic comes from sip_strtouint(), so SIP already behaves this way,
but IRC did not. Real DCC clients probably don't zero-pad ports like
this. Could the helper documentation, or this commit message, mention
the limit?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260923091608.2617604-1-rc%40rexion.ai

Re: [PATCH nf-next v4 3/4] netfilter: nf_conntrack_amanda: use nf_ct_helper_parse_port()

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] amanda_help() in net/netfilter/nf_conntrack_amanda.c still
  silently cuts a port field of six or more digits down to a different,…

--

netfilter: nf_conntrack_amanda: use nf_ct_helper_parse_port()

This patch replaces the simple_strtoul() + htons() port parsing in
amanda_help() with nf_ct_helper_parse_port(). The goal is to reject port 0
and values above 65535 instead of silently truncating them to a u16. The
NAT mangle length is still taken from the parser's end pointer.

> diff --git a/net/netfilter/nf_conntrack_amanda.c b/net/netfilter/nf_conntrack_amanda.c
> index 14ae660491f36..f157c96b7d223 100644
> --- a/net/netfilter/nf_conntrack_amanda.c
> +++ b/net/netfilter/nf_conntrack_amanda.c
> @@ -89,7 +89,7 @@ static int amanda_help(struct sk_buff *skb,
>  	struct nf_conntrack_tuple *tuple;
>  	unsigned int dataoff, start, stop, off, i;
>  	char pbuf[sizeof("65535")], *tmp;
> -	u16 len;
> +	u16 len, parsed_port;
>  	__be16 port;
>  	int ret = NF_ACCEPT;
>  	nf_nat_amanda_hook_fn *nf_nat_amanda;
> @@ -132,10 +132,10 @@ static int amanda_help(struct sk_buff *skb,
>  			break;
>  		pbuf[len] = '\0';
>  
> -		port = htons(simple_strtoul(pbuf, &tmp, 10));
> -		len = tmp - pbuf;
> -		if (port == 0 || len > 5)
> +		if (nf_ct_helper_parse_port(pbuf, len, &parsed_port, &tmp))
>  			break;
> +		port = htons(parsed_port);
> +		len = tmp - pbuf;

[Severity: High]
Can a port field with six or more digits still be truncated into a
different valid port here?

pbuf is only 6 bytes, and just before this hunk amanda_help() caps the
copy at 5 bytes:

	len = min_t(unsigned int, sizeof(pbuf) - 1, stop - off);
	if (skb_copy_bits(skb, off, pbuf, len))
		break;
	pbuf[len] = '\0';

So nf_ct_helper_parse_port(pbuf, 5, ...) only ever sees the first five
digits. The range check there only covers the len bytes it is given:

net/netfilter/nf_conntrack_helper.c:nf_ct_helper_parse_port() {
	val = nf_ct_helper_parse_uint(cp, len, &e);
	if (e == cp || val == 0 || val > 65535)
		return -1;
}

Nothing checks whether skb byte off + len is another digit. So a reply
such as "CONNECT ... DATA 123456\n" would be accepted as port 12345. In
the same way, 100000 would become 10000 and 655350 would become 65535.
nf_ct_expect_init() would then install a TCP expectation for a port that
never appeared on the wire.

On the NAT path, len = tmp - pbuf is 5 and is passed as matchlen.
help() in net/netfilter/nf_nat_amanda.c then does:

	if (!nf_nat_mangle_udp_packet(skb, ct, ctinfo,
				      protoff, matchoff, matchlen,
				      buffer, strlen(buffer))) {

Only the first 5 bytes get replaced, so the extra digit stays in the
payload. For example, a NAT-chosen port of 40000 would come out as
"400006", which no longer matches the expectation.

The commit message says:

    The digit string is copied into a 6-byte buffer,
    so at most five digits are parsed, but values 65536-99999 still truncated

and:

    Use nf_ct_helper_parse_port(), which rejects port 0 and values above
    65535 in one call.

With the 5-byte window, is that accurate? Values above 99999 are also
above 65535, and they still truncate through the window instead of being
rejected. The later commit "netfilter: nf_conntrack_sip: use shared
helper parsers" in this series does not touch amanda, so this code is
unchanged at the end of the series.

Would it work to copy one extra byte into pbuf, or to check that skb
byte off + len is not a digit, and reject the field when the digit run
is longer than five characters?

>  
>  		exp = nf_ct_expect_alloc(ct);
>  		if (exp == NULL) {

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260923091608.2617604-1-rc%40rexion.ai

Re: [PATCH nf-next v4 4/4] netfilter: nf_conntrack_sip: use shared helper parsers

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The commit message says "No functional change intended", but
  the patch changes how sip_parse_port() in…

--

netfilter: nf_conntrack_sip: use shared helper parsers

This patch removes the SIP helper's private sip_strtouint() and switches
its callers to nf_ct_helper_parse_uint(). The digit loop in
sip_parse_port() is replaced with nf_ct_helper_parse_port(), while the
SIP-specific handling of ':', the SIP_PORT default and the 1024 minimum
stay in place. The commit message says no functional change is intended.

> diff --git a/net/netfilter/nf_conntrack_sip.c b/net/netfilter/nf_conntrack_sip.c
> index 64bc440b1181f..0e12426227a80 100644
> --- a/net/netfilter/nf_conntrack_sip.c
> +++ b/net/netfilter/nf_conntrack_sip.c

[ ... ]

> @@ -199,29 +199,22 @@ static bool sip_parse_port(const char *dptr, const char **endp,
>  
>  	dptr++; /* skip ':' */
>  
> -	while (dptr < limit && isdigit(*dptr)) {
> -		p = p * 10 + (*dptr - '0');
> -		dptr++;
> -		len++;
> -		if (len > 5) /* max "65535" */
> -			return false;
> -	}
> -
> -	if (len == 0)
> +	if (nf_ct_helper_parse_port(dptr, limit - dptr, &p, &end))
>  		return false;

[Severity: Medium]
Does this change which ports sip_parse_port() accepts?

The old loop counted digits and returned false once there were more than
five, whatever the value. That rejected zero-padded ports such as
"sip:1.2.3.4:0005060".

The shared helper only checks the value:

net/netfilter/nf_conntrack_helper.c:nf_ct_helper_parse_port() {
    ...
	val = nf_ct_helper_parse_uint(cp, len, &e);
	if (e == cp || val == 0 || val > 65535)
		return -1;
    ...
}

nf_ct_helper_parse_uint() caps the digit count only at
sizeof("4294967295"), which is 11. As a result, ports of 6 to 11 digits
with leading zeros, such as ":0005060" or ":00000065535", now parse.

That appears to change the outcome for the same packet in several
callers:

  - ct_sip_parse_header_uri() used to return -1 for these strings, and
    process_register_request() and process_register_response() then
    returned NF_DROP with "cannot parse contact". It now returns 1, so a
    REGISTER request creates a signalling expectation:

    sip_help_udp/tcp()->process_sip_msg()->process_sip_request()->
      process_register_request()->ct_sip_parse_header_uri()->
        sip_parse_port()

  - ct_sip_parse_request() used to return -1 and now succeeds.

  - epaddr_len() and skp_epaddr_len(), the match_len callbacks used by
    ct_sip_get_header() and ct_sip_walk_headers(), used to return 0,
    which made the lookup return -1. Now the header matches with a
    longer matchlen, and nf_nat_sip mangles that range.

  - process_sip_request() may now set ct_sip_info->forced_dport from a
    zero-padded Via header that it used to ignore.

The sip_strtouint() to nf_ct_helper_parse_uint() swap looks identical.
The sip_parse_port() conversion is not, and the commit message quoted
below does not mention it:

  "No functional change intended."

Should the old limit be kept, for example by rejecting end - dptr > 5?
Otherwise, could the commit message document that leading zeros are now
accepted?

>  
>  	/* reached limit while parsing port */
> -	if (dptr >= limit)
> +	if (end >= limit)
>  		return false;

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260923091608.2617604-1-rc%40rexion.ai

Re: [PATCH nf-next v4 3/4] netfilter: nf_conntrack_amanda: use nf_ct_helper_parse_port()

[Rahul Chandelkar]

On Sun, 27 Sep 2026 17:37:03 +0000, netdev-bot+sashiko@kernel.org wrote:
> Can a port field with six or more digits still be truncated into a
> different valid port here?

Yes.  pbuf only holds five digits, so a field such as 123456 is parsed
as 12345, and on the NAT path only its first five bytes are mangled.
This predates the series, but the commit message wrongly claims that
the patch rejects every value above 65535.

v5 will make pbuf one byte larger so that a sixth digit is copied and
the field is rejected by nf_ct_helper_parse_port().

Thanks,
Rahul

pw-bot: cr

Re: [PATCH nf-next v4 4/4] netfilter: nf_conntrack_sip: use shared helper parsers

[Rahul Chandelkar]

On Sun, 27 Sep 2026 17:37:05 +0000, netdev-bot+sashiko@kernel.org wrote:
> Should the old limit be kept, for example by rejecting end - dptr > 5?

Yes, v4 did change behaviour here: the old loop rejected zero-padded
ports such as :0005060 and v4 accepts them.  In v5
nf_ct_helper_parse_port() will reject digit runs longer than five
characters, which restores the old behaviour.  The old and new
sip_parse_port() give identical results on three million random inputs
in a userspace harness built from the kernel source.

Thanks,
Rahul

Re: [PATCH nf-next v4 1/4] netfilter: conntrack: add shared uint and port parsers for helpers

[Rahul Chandelkar]

On Sun, 27 Sep 2026 17:37:01 +0000, netdev-bot+sashiko@kernel.org wrote:
> Could the digit-count limit be dropped, or else documented in both
> comments?

I will keep the limit and document it.  It comes from sip_strtouint(),
so dropping it would change what the SIP helper accepts.  In v5 both
parsers will get kernel-doc that states their digit limits, and
nf_ct_helper_parse_port() will reject any digit run longer than five
characters.

Thanks,
Rahul

Re: [PATCH nf-next v4 2/4] netfilter: nf_conntrack_irc: use nf_ct_helper_parse_port()

[Rahul Chandelkar]

On Sun, 27 Sep 2026 17:37:02 +0000, netdev-bot+sashiko@kernel.org wrote:
> Should that clause be removed, with the logging change mentioned in the
> commit message?

Yes.  v5 will remove the dcc_port == 0 test, and its commit message
will say that a DCC command with port 0 now takes the "unable to parse
dcc command" debug path instead of the "Forged DCC command" warning.

> Could the helper documentation, or this commit message, mention
> the limit?

Yes, both will in v5.

Thanks,
Rahul