mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH nf] ipvs: avoid wrapping persistent service timeouts
@ 2026-10-31  7:15 Yingjie Wang
  2026-10-31 20:13 ` Julian Anastasov
  0 siblings, 1 reply; 3+ messages in thread
From: Yingjie Wang @ 2026-10-31  7:15 UTC (permalink / raw)
  To: lvs-devel, netdev
  Cc: horms, ja, pablo, fw, phil, davem, edumazet, kuba, pabeni,
	netfilter-devel, coreteam, linux-kernel

ip_vs_add_service() and ip_vs_edit_service() convert the persistent
timeout from seconds to jiffies with u->timeout * HZ. Both operands are
unsigned int and the value comes unchecked from the socket API or from
IPVS_SVC_ATTR_TIMEOUT, so timeouts above UINT_MAX / HZ wrap.

With HZ=1000, 4294968 seconds becomes 704 jiffies and is reported back
as 0 seconds. Persistence templates copy svc->timeout, so their timer is
armed for 0.7 seconds and a client can be moved to another real server
long before the configured timeout.

A related boundary exists on 32-bit kernels. Some products that fit in
unsigned int exceed the maximum safe jiffy delay. For example, 4294967
seconds becomes 4294967000 jiffies with HZ=1000. Adding that delay to
jiffies wraps, and the timer code can treat the result as already expired.

Limit the seconds value using both the storage width and
MAX_JIFFY_OFFSET before multiplying by HZ. Saturate out-of-range values
instead of rejecting requests that previously succeeded. Also print the
stored unsigned timeout with %u in /proc/net/ip_vs, since a valid stored
value can exceed INT_MAX.

The overflow was reproduced in an x86-64 QEMU guest (HZ=1000) through
the IPVS socket API. A persistent one-packet UDP service with two real
servers was set to 4294968 seconds, and a client in another network
namespace sent two datagrams 1.5 seconds apart. On the base kernel the
template expired between the datagrams, as it did for a 1-second
control, and the second datagram went to the other real server. With
this change the template remained present and both datagrams went to the
same server. The 300-second control was unchanged.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Yingjie Wang <x-professor@outlook.com>
---

Notes:
    ip_vs_set_timeout() rejects global TCP/UDP timeouts above INT_MAX / HZ
    with -EINVAL. This path saturates instead because the persistent timeout
    is a __u32 whose full range has always been accepted. Saturation avoids
    adding a new error result while keeping the stored delay within both the
    storage width and the safe jiffy range.

 net/netfilter/ipvs/ip_vs_ctl.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index ce9fe20d6106a..0d4a113eb5a7f 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -1749,6 +1749,15 @@ static void ip_vs_dest_trash_expire(struct timer_list *t)
 	spin_unlock(&ipvs->dest_trash_lock);
 }
 
+/* Convert seconds without overflowing or exceeding a safe jiffy delay. */
+static unsigned int ip_vs_svc_timeout(unsigned int timeout)
+{
+	unsigned long max_secs;
+
+	max_secs = min_t(unsigned long, UINT_MAX, MAX_JIFFY_OFFSET) / HZ;
+	return min_t(unsigned long, timeout, max_secs) * HZ;
+}
+
 /*
  *	Add a service into the service hash table
  */
@@ -1864,7 +1873,7 @@ ip_vs_add_service(struct netns_ipvs *ipvs, struct ip_vs_service_user_kern *u,
 	svc->port = u->port;
 	svc->fwmark = u->fwmark;
 	svc->flags = u->flags & ~IP_VS_SVC_F_HASHED;
-	svc->timeout = u->timeout * HZ;
+	svc->timeout = ip_vs_svc_timeout(u->timeout);
 	svc->netmask = u->netmask;
 	svc->ipvs = ipvs;
 
@@ -2031,7 +2040,7 @@ ip_vs_edit_service(struct ip_vs_service *svc, struct ip_vs_service_user_kern *u)
 	 * Set the flags and timeout value
 	 */
 	svc->flags = u->flags | IP_VS_SVC_F_HASHED;
-	svc->timeout = u->timeout * HZ;
+	svc->timeout = ip_vs_svc_timeout(u->timeout);
 	svc->netmask = u->netmask;
 
 	old_pe = rcu_dereference_protected(svc->pe, 1);
@@ -3064,9 +3073,9 @@ static int ip_vs_info_seq_show(struct seq_file *seq, void *v)
 		}
 
 		if (svc->flags & IP_VS_SVC_F_PERSISTENT)
-			seq_printf(seq, "persistent %d %08X\n",
-				svc->timeout,
-				ntohl(svc->netmask));
+			seq_printf(seq, "persistent %u %08X\n",
+				   svc->timeout,
+				   ntohl(svc->netmask));
 		else
 			seq_putc(seq, '\n');
 

base-commit: 734bef548a96e8774d6eef5dfa2026c6edc4e80b
-- 
2.43.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH nf] ipvs: avoid wrapping persistent service timeouts
  2026-10-31  7:15 [PATCH nf] ipvs: avoid wrapping persistent service timeouts Yingjie Wang
@ 2026-10-31 20:13 ` Julian Anastasov
  0 siblings, 0 replies; 3+ messages in thread
From: Julian Anastasov @ 2026-10-31 20:13 UTC (permalink / raw)
  To: Yingjie Wang
  Cc: lvs-devel, netdev, horms, pablo, fw, phil, davem, edumazet, kuba,
	pabeni, netfilter-devel, coreteam, linux-kernel


	Hello,

On Sat, 31 Oct 2026, Yingjie Wang wrote:

> ip_vs_add_service() and ip_vs_edit_service() convert the persistent
> timeout from seconds to jiffies with u->timeout * HZ. Both operands are
> unsigned int and the value comes unchecked from the socket API or from
> IPVS_SVC_ATTR_TIMEOUT, so timeouts above UINT_MAX / HZ wrap.
> 
> With HZ=1000, 4294968 seconds becomes 704 jiffies and is reported back
> as 0 seconds. Persistence templates copy svc->timeout, so their timer is
> armed for 0.7 seconds and a client can be moved to another real server
> long before the configured timeout.
> 
> A related boundary exists on 32-bit kernels. Some products that fit in
> unsigned int exceed the maximum safe jiffy delay. For example, 4294967
> seconds becomes 4294967000 jiffies with HZ=1000. Adding that delay to
> jiffies wraps, and the timer code can treat the result as already expired.
> 
> Limit the seconds value using both the storage width and
> MAX_JIFFY_OFFSET before multiplying by HZ. Saturate out-of-range values
> instead of rejecting requests that previously succeeded. Also print the
> stored unsigned timeout with %u in /proc/net/ip_vs, since a valid stored
> value can exceed INT_MAX.
> 
> The overflow was reproduced in an x86-64 QEMU guest (HZ=1000) through
> the IPVS socket API. A persistent one-packet UDP service with two real
> servers was set to 4294968 seconds, and a client in another network
> namespace sent two datagrams 1.5 seconds apart. On the base kernel the
> template expired between the datagrams, as it did for a 1-second
> control, and the second datagram went to the other real server. With
> this change the template remained present and both datagrams went to the
> same server. The 300-second control was unchanged.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Yingjie Wang <x-professor@outlook.com>
> ---
> 
> Notes:
>     ip_vs_set_timeout() rejects global TCP/UDP timeouts above INT_MAX / HZ
>     with -EINVAL. This path saturates instead because the persistent timeout
>     is a __u32 whose full range has always been accepted. Saturation avoids
>     adding a new error result while keeping the stored delay within both the
>     storage width and the safe jiffy range.
> 
>  net/netfilter/ipvs/ip_vs_ctl.c | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index ce9fe20d6106a..0d4a113eb5a7f 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -1749,6 +1749,15 @@ static void ip_vs_dest_trash_expire(struct timer_list *t)
>  	spin_unlock(&ipvs->dest_trash_lock);
>  }
>  
> +/* Convert seconds without overflowing or exceeding a safe jiffy delay. */
> +static unsigned int ip_vs_svc_timeout(unsigned int timeout)
> +{
> +	unsigned long max_secs;
> +
> +	max_secs = min_t(unsigned long, UINT_MAX, MAX_JIFFY_OFFSET) / HZ;
> +	return min_t(unsigned long, timeout, max_secs) * HZ;
> +}
> +

	Better to add something like this in include/net/ip_vs.h,
it will be used in multiple files:

/* All timeouts up to 31 days */
#define IP_VS_TIMEOUT_MAX_SECS	(86400 * 31)

static inline unsigned long ip_vs_timeout_ticks(unsigned int secs)
{
	secs = min_t(unsigned int, secs, IP_VS_TIMEOUT_MAX_SECS);
#if BITS_PER_LONG <= 32
	secs = min_t(unsigned int, secs, MAX_JIFFY_OFFSET / HZ);
#endif
	return (unsigned long) secs * HZ;
}

	The IP_VS_TIMEOUT_MAX_SECS value comes from the ipvsadm
MAX_TIMEOUT. It is used both for service persistence and the
protocol timeouts. We have to add checks in ip_vs_add_service and
ip_vs_edit_service for IP_VS_TIMEOUT_MAX_SECS and to return -EINVAL.
As the timeout field in struct ip_vs_service is unsigned int, it
should be changed to unsigned long, it is in ticks. The checks in 
ip_vs_set_timeout() should be for IP_VS_TIMEOUT_MAX_SECS, not for
INT_MAX / HZ.

	Then when we convert the secs to ticks we should use
ticks = ip_vs_timeout_ticks(secs). This includes svc->timeout and
the assignments in ip_vs_set_timeout().

	Also, ip_vs_proc_conn() in ip_vs_sync.c should use
cp->timeout = ip_vs_timeout_ticks(timeout) too, instead of
MAX_SCHEDULE_TIMEOUT / HZ * HZ. This place is the reason we
to add the first line in ip_vs_timeout_ticks(), the other
places will have early check for IP_VS_TIMEOUT_MAX_SECS.

	The idea is to reject timeouts above 31 days but
to silently clamp the value to MAX_JIFFY_OFFSET / HZ due to
32-bit platform limits.

>  /*
>   *	Add a service into the service hash table
>   */
> @@ -1864,7 +1873,7 @@ ip_vs_add_service(struct netns_ipvs *ipvs, struct ip_vs_service_user_kern *u,
>  	svc->port = u->port;
>  	svc->fwmark = u->fwmark;
>  	svc->flags = u->flags & ~IP_VS_SVC_F_HASHED;
> -	svc->timeout = u->timeout * HZ;
> +	svc->timeout = ip_vs_svc_timeout(u->timeout);
>  	svc->netmask = u->netmask;
>  	svc->ipvs = ipvs;
>  
> @@ -2031,7 +2040,7 @@ ip_vs_edit_service(struct ip_vs_service *svc, struct ip_vs_service_user_kern *u)
>  	 * Set the flags and timeout value
>  	 */
>  	svc->flags = u->flags | IP_VS_SVC_F_HASHED;
> -	svc->timeout = u->timeout * HZ;
> +	svc->timeout = ip_vs_svc_timeout(u->timeout);
>  	svc->netmask = u->netmask;
>  
>  	old_pe = rcu_dereference_protected(svc->pe, 1);
> @@ -3064,9 +3073,9 @@ static int ip_vs_info_seq_show(struct seq_file *seq, void *v)
>  		}
>  
>  		if (svc->flags & IP_VS_SVC_F_PERSISTENT)
> -			seq_printf(seq, "persistent %d %08X\n",
> -				svc->timeout,
> -				ntohl(svc->netmask));
> +			seq_printf(seq, "persistent %u %08X\n",
> +				   svc->timeout,
> +				   ntohl(svc->netmask));
>  		else
>  			seq_putc(seq, '\n');
>  
> 
> base-commit: 734bef548a96e8774d6eef5dfa2026c6edc4e80b
> -- 
> 2.43.0

Regards

--
Julian Anastasov <ja@ssi.bg>


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH nf] ipvs: avoid wrapping persistent service timeouts
@ 2026-10-31  7:18 netdev-bot+sinfo
  0 siblings, 0 replies; 3+ messages in thread
From: netdev-bot+sinfo @ 2026-10-31  7:18 UTC (permalink / raw)
  To: Yingjie Wang
  Cc: lvs-devel, netdev, horms, ja, pablo, fw, phil, davem, edumazet,
	kuba, pabeni, netfilter-devel, coreteam, linux-kernel

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.

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.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-10-31 20:13 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-31  7:15 [PATCH nf] ipvs: avoid wrapping persistent service timeouts Yingjie Wang
2026-10-31 20:13 ` Julian Anastasov
2026-10-31  7:18 netdev-bot+sinfo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®