[PATCH v4] netfilter: conntrack: fix integer overflow in expectation timeout

[PATCH v4] netfilter: conntrack: fix integer overflow in expectation timeout

[Àlex Fernández]

In ctnetlink_change_expect(), the expectation timeout is calculated by
multiplying the user-provided timeout value by HZ. Because ntohl()
returns a 32-bit unsigned integer, this multiplication is performed in
32-bit arithmetic before being promoted to the 64-bit jiffies format.

If a user provides a large enough timeout (e.g., 42949673 on a system
with HZ=100), the multiplication wraps around the 32-bit limit,
resulting in a near-zero jiffies value. This causes the expectation
to be immediately collected by the garbage collector instead of staying
open for the requested duration.

This patch casts the result of ntohl() to u64 prior to multiplication,
matching the safe pattern already used for standard conntrack timeouts.

Signed-off-by: Àlex Fernández <tomaquet18@protonmail.com>
---
 net/netfilter/nf_conntrack_netlink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index eda5fe4a7..be89bf1ba 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -3466,7 +3466,7 @@ ctnetlink_change_expect(struct nf_conntrack_expect *x,
 			return -ETIME;
 
 		x->timeout.expires = jiffies +
-			ntohl(nla_get_be32(cda[CTA_EXPECT_TIMEOUT])) * HZ;
+			(u64)ntohl(nla_get_be32(cda[CTA_EXPECT_TIMEOUT])) * HZ;
 		add_timer(&x->timeout);
 	}
 	return 0;
-- 
2.43.0

Re: [PATCH v4] netfilter: conntrack: fix integer overflow in expectation timeout

[Florian Westphal]

Àlex Fernández <tomaquet18@protonmail.com> wrote:
>  		x->timeout.expires = jiffies +
> -			ntohl(nla_get_be32(cda[CTA_EXPECT_TIMEOUT])) * HZ;
> +			(u64)ntohl(nla_get_be32(cda[CTA_EXPECT_TIMEOUT])) * HZ;

https://sashiko.dev/#/patchset/20260504112300.715192-1-tomaquet18%40protonmail.com

Does this fully resolve the overflow on 32-bit architectures?
The expires field in struct timer_list is an unsigned long, which is 32 bits
wide on 32-bit systems. Assigning the 64-bit multiplication result directly
to expires will silently truncate it back to 32 bits, causing the same
wraparound this patch intends to fix.
Additionally, does providing a timeout delta larger than INT_MAX break the
kernel's signed timer comparisons?
If the delta exceeds INT_MAX, macros like time_after() will evaluate the
timer as being in the past, causing it to expire immediately.
Should the computed timeout delta be explicitly clamped to a safe maximum
(such as INT_MAX or MAX_JIFFY_OFFSET), similar to the logic used for
standard conntrack timeouts?

Re: [PATCH v4] netfilter: conntrack: fix integer overflow in expectation timeout

[Pablo Neira Ayuso]

On Tue, May 19, 2026 at 09:38:12PM +0200, Florian Westphal wrote:
> Àlex Fernández <tomaquet18@protonmail.com> wrote:
> >  		x->timeout.expires = jiffies +
> > -			ntohl(nla_get_be32(cda[CTA_EXPECT_TIMEOUT])) * HZ;
> > +			(u64)ntohl(nla_get_be32(cda[CTA_EXPECT_TIMEOUT])) * HZ;

Yes, for correctness, fixing this is fine but...

> https://sashiko.dev/#/patchset/20260504112300.715192-1-tomaquet18%40protonmail.com
> 
> Does this fully resolve the overflow on 32-bit architectures?
> The expires field in struct timer_list is an unsigned long, which is 32 bits
> wide on 32-bit systems. Assigning the 64-bit multiplication result directly
> to expires will silently truncate it back to 32 bits, causing the same
> wraparound this patch intends to fix.
> Additionally, does providing a timeout delta larger than INT_MAX break the
> kernel's signed timer comparisons?
> If the delta exceeds INT_MAX, macros like time_after() will evaluate the
> timer as being in the past, causing it to expire immediately.

the submitter claims you can create expectations that expires
inmediately, but what is the issue with this?

> Should the computed timeout delta be explicitly clamped to a safe maximum
> (such as INT_MAX or MAX_JIFFY_OFFSET), similar to the logic used for
> standard conntrack timeouts?

I think this is just a cleanup / nf-next material?

Re: [PATCH v4] netfilter: conntrack: fix integer overflow in expectation timeout

[Pablo Neira Ayuso]

On Tue, May 19, 2026 at 09:55:06PM +0200, Pablo Neira Ayuso wrote:
> On Tue, May 19, 2026 at 09:38:12PM +0200, Florian Westphal wrote:
> > Àlex Fernández <tomaquet18@protonmail.com> wrote:
> > >  		x->timeout.expires = jiffies +
> > > -			ntohl(nla_get_be32(cda[CTA_EXPECT_TIMEOUT])) * HZ;
> > > +			(u64)ntohl(nla_get_be32(cda[CTA_EXPECT_TIMEOUT])) * HZ;
> 
> Yes, for correctness, fixing this is fine but...
> 
> > https://sashiko.dev/#/patchset/20260504112300.715192-1-tomaquet18%40protonmail.com
> > 
> > Does this fully resolve the overflow on 32-bit architectures?
> > The expires field in struct timer_list is an unsigned long, which is 32 bits
> > wide on 32-bit systems. Assigning the 64-bit multiplication result directly
> > to expires will silently truncate it back to 32 bits, causing the same
> > wraparound this patch intends to fix.
> > Additionally, does providing a timeout delta larger than INT_MAX break the
> > kernel's signed timer comparisons?
> > If the delta exceeds INT_MAX, macros like time_after() will evaluate the
> > timer as being in the past, causing it to expire immediately.
> 
> the submitter claims you can create expectations that expires
> inmediately, but what is the issue with this?
> 
> > Should the computed timeout delta be explicitly clamped to a safe maximum
> > (such as INT_MAX or MAX_JIFFY_OFFSET), similar to the logic used for
> > standard conntrack timeouts?
> 
> I think this is just a cleanup / nf-next material?

But this can also go through nf.git for correctness, we can just
describe in the PR the effect of this bug.

Re: [PATCH v4] netfilter: conntrack: fix integer overflow in expectation timeout

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > I think this is just a cleanup / nf-next material?
> 
> But this can also go through nf.git for correctness, we can just
> describe in the PR the effect of this bug.

Yes, its low risk change to clamp this, it can go via nf too.