[PATCH net 1/2] netfilter: conntrack: avoid secs-to-jiffies overflow in SIP helper

[PATCH net 1/2] netfilter: conntrack: avoid secs-to-jiffies overflow in SIP helper

[luoqing]

From: Qing Luo <luoqing@kylinos.cn>

refresh_signalling_expectation() converts the expiry from a SIP
REGISTER 2xx response with "expires * HZ" in 32-bit arithmetic and
stores it in a u32.  sip_strtouint() only rejects values above
UINT_MAX, so the answering registrar controls the input.

Expectations are reaped with "(__s32)(exp->timeout - nfct_time_stamp)
<= 0".  At HZ=1000 an expiry of 2147484 seconds (24.9 days) crosses
2^31 and the expectation is born expired, while 4294968 seconds wraps
to 704 jiffies.  Registration refreshes break, or a signalling
expectation stays pinned for days.

Add nf_ct_secs_to_jiffies(), which multiplies in u64 and saturates at
INT_MAX so the signed expiry test stays positive, and use it for the
SIP expectation refresh.

Sweeping all 2^32-1 inputs at HZ=1000, the helper never wraps, never
yields a value that reads as expired, and matches the old expression
up to INT_MAX / HZ.

Fixes: 0f32a40fc91a ("[NETFILTER]: nf_conntrack_sip: create signalling expectations")
Cc: stable@vger.kernel.org
Signed-off-by: Qing Luo <luoqing@kylinos.cn>
---
 include/net/netfilter/nf_conntrack.h | 10 ++++++++++
 net/netfilter/nf_conntrack_sip.c     |  3 ++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/include/net/netfilter/nf_conntrack.h b/include/net/netfilter/nf_conntrack.h
index c39425e54d87..b32716084ea8 100644
--- a/include/net/netfilter/nf_conntrack.h
+++ b/include/net/netfilter/nf_conntrack.h
@@ -210,6 +210,16 @@ bool nf_ct_get_tuplepr(const struct sk_buff *skb, unsigned int nhoff,
 void __nf_ct_refresh_acct(struct nf_conn *ct, enum ip_conntrack_info ctinfo,
 			  u32 extra_jiffies, unsigned int bytes);
 
+/* Convert a timeout in seconds to jiffies, saturating at INT_MAX instead of
+ * wrapping the "secs * HZ" product in u32.
+ */
+static inline u32 nf_ct_secs_to_jiffies(u32 secs)
+{
+	u64 delta = (u64)secs * HZ;
+
+	return delta > INT_MAX ? INT_MAX : (u32)delta;
+}
+
 /* Refresh conntrack for this many jiffies and do accounting */
 static inline void nf_ct_refresh_acct(struct nf_conn *ct,
 				      enum ip_conntrack_info ctinfo,
diff --git a/net/netfilter/nf_conntrack_sip.c b/net/netfilter/nf_conntrack_sip.c
index 64bc440b1181..e2fd97f53386 100644
--- a/net/netfilter/nf_conntrack_sip.c
+++ b/net/netfilter/nf_conntrack_sip.c
@@ -891,7 +891,8 @@ static int refresh_signalling_expectation(struct nf_conn *ct,
 		    exp->tuple.dst.protonum != proto ||
 		    exp->tuple.dst.u.udp.port != port)
 			continue;
-		WRITE_ONCE(exp->timeout, nfct_time_stamp + (expires * HZ));
+		WRITE_ONCE(exp->timeout,
+			   nfct_time_stamp + nf_ct_secs_to_jiffies(expires));
 		WRITE_ONCE(exp->flags, exp->flags & ~NF_CT_EXPECT_INACTIVE);
 		found = 1;
 		break;
-- 
2.25.1

[PATCH net 2/2] netfilter: h323: avoid secs-to-jiffies overflow in RAS timeout

[luoqing]

From: Qing Luo <luoqing@kylinos.cn>

process_rcf() converts info->timeout with "* HZ" twice, once for the
conntrack refresh and once for the Q.931 expectation, both in 32-bit
arithmetic.  info->timeout is a u32 decoded from the ASN.1 timeToLive
field and only checked for being non-zero, so it is remote-controlled
by both the endpoint (RegistrationRequest) and the gatekeeper
(RegistrationConfirm).

With the signed expiry test "(__s32)(ct->timeout - nfct_time_stamp)
<= 0", a timeout of 2147484 seconds (24.9 days) at HZ=1000 is born
expired, and 4294968 seconds wraps to 704 jiffies.  The RAS connection
therefore drops mid-call, or the Q.931 expectation stays pinned for
days after the peer is gone.

Reuse nf_ct_secs_to_jiffies() for both conversions and compute the
value once.

Sweeping all 2^32-1 inputs at HZ=1000, the helper never wraps, never
yields a value that reads as expired, and matches the old expression
up to INT_MAX / HZ; objdump shows the two 32-bit "imul $0x3e8" in
ras_help() replaced by a 64-bit multiply plus cmp/cmova saturation.

Fixes: f587de0e2feb ("[NETFILTER]: nf_conntrack/nf_nat: add H.323 helper port")
Cc: stable@vger.kernel.org
Signed-off-by: Qing Luo <luoqing@kylinos.cn>
---
 net/netfilter/nf_conntrack_h323_main.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/netfilter/nf_conntrack_h323_main.c b/net/netfilter/nf_conntrack_h323_main.c
index 4cb1665bba02..5e900037fa6f 100644
--- a/net/netfilter/nf_conntrack_h323_main.c
+++ b/net/netfilter/nf_conntrack_h323_main.c
@@ -1375,9 +1375,11 @@ static int process_rcf(struct sk_buff *skb, struct nf_conn *ct,
 	}
 
 	if (info->timeout > 0) {
+		u32 timeout = nf_ct_secs_to_jiffies(info->timeout);
+
 		pr_debug("nf_ct_ras: set RAS connection timeout to "
 			 "%u seconds\n", info->timeout);
-		nf_ct_refresh(ct, info->timeout * HZ);
+		nf_ct_refresh(ct, timeout);
 
 		/* Set expect timeout */
 		spin_lock_bh(&nf_conntrack_expect_lock);
@@ -1388,8 +1390,7 @@ static int process_rcf(struct sk_buff *skb, struct nf_conn *ct,
 				 "timeout to %u seconds for",
 				 info->timeout);
 			nf_ct_dump_tuple(&exp->tuple);
-			WRITE_ONCE(exp->timeout,
-				   nfct_time_stamp + (info->timeout * HZ));
+			WRITE_ONCE(exp->timeout, nfct_time_stamp + timeout);
 		}
 		spin_unlock_bh(&nf_conntrack_expect_lock);
 	}
-- 
2.25.1

Re: [PATCH net 1/2] netfilter: conntrack: avoid secs-to-jiffies overflow in SIP helper

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.