Re: [PATCH nf] ipvs: avoid wrapping persistent service timeouts

[Julian Anastasov <ja@ssi.bg>]

	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>