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

[Julian Anastasov]

	Hello,

On Sun, 1 Nov 2026, Yingjie Wang wrote:

> ip_vs_add_service() and ip_vs_edit_service() convert an unchecked u32
> timeout from seconds to jiffies with u->timeout * HZ. Both operands are
> unsigned int, so values 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.
> 
> ipvsadm limits service persistence and protocol timeouts to 31 days.
> Enforce the same limit in the kernel and reject larger values with
> -EINVAL. Convert accepted values through a common helper that clamps
> them to MAX_JIFFY_OFFSET / HZ on 32-bit systems. Use the helper for
> timeouts received from the synchronization protocol as well, since those
> values do not pass through the service configuration checks.
> 
> Change the service timeout and protocol timeout tables to unsigned long
> because they store jiffies. With HZ=1000, 31 days is 2678400000 jiffies,
> which does not fit in the signed int used by the protocol tables. This
> also allows the global protocol timeout interface to represent the full
> 31-day range on 64-bit systems.
> 
> The overflow was reproduced in an x86-64 QEMU guest with HZ=1000. A
> persistent one-packet UDP service was set to 4294968 seconds, and two
> datagrams sent 1.5 seconds apart were directed to different real servers
> after the template expired.
> 
> On the x86-64 test kernel with HZ=1000, 2678401-second service timeouts
> were accepted and 4294968 seconds read back as 0, while a 2678400-second
> protocol timeout was rejected. With this change, 2678401 and 4294968
> seconds are rejected, a failed edit leaves the previous service timeout
> unchanged, and 2678400-second service and protocol timeouts read back
> unchanged.
> 
> On an i386 test kernel with HZ=1000, the accepted 2678400-second values
> were clamped to 1073741 seconds by the 32-bit jiffy limit and read back
> as such through both interfaces.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Suggested-by: Julian Anastasov <ja@xxxxxx>
> Link: https://lore.kernel.org/r/4f420fda8690ac470be44b3da284b65501c77bb5.1793430892.git.x-professor@xxxxxxxxxxx
> Link: https://lore.kernel.org/r/5fa37fba-530c-2c5b-038e-9cb95e34023d@xxxxxx
> Signed-off-by: Yingjie Wang <x-professor@xxxxxxxxxxx>
> ---
> 
> Notes:
>     Changes since v1:
>     - Follow Julian's suggestion to use the 31-day ipvsadm limit, reject
>       larger configured values, and clamp accepted values only for the
>       32-bit jiffy limit.
>     - Move the conversion helper to include/net/ip_vs.h and use it for
>       service, protocol and synchronized connection timeouts.
>     - Store service and protocol timeout ticks in unsigned long values so
>       the full range is representable.
>     - Add runtime coverage for the 31-day boundary, failed service updates
>       and global TCP/UDP timeout readback on x86-64 and i386. The i386 test
>       also verifies the intended MAX_JIFFY_OFFSET clamp.
>     
>     v1: https://lore.kernel.org/r/4f420fda8690ac470be44b3da284b65501c77bb5.1793430892.git.x-professor@xxxxxxxxxxx
> 
>  include/net/ip_vs.h                   | 20 ++++++++++++---
>  net/netfilter/ipvs/ip_vs_app.c        |  3 +--
>  net/netfilter/ipvs/ip_vs_ctl.c        | 35 ++++++++++++++++-----------
>  net/netfilter/ipvs/ip_vs_proto.c      |  4 +--
>  net/netfilter/ipvs/ip_vs_proto_sctp.c |  6 ++---
>  net/netfilter/ipvs/ip_vs_proto_tcp.c  |  6 ++---
>  net/netfilter/ipvs/ip_vs_proto_udp.c  |  6 ++---
>  net/netfilter/ipvs/ip_vs_sync.c       |  6 ++---
>  8 files changed, 52 insertions(+), 34 deletions(-)
> 
> diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
> index 9a83bff32569..4eaa06b5f991 100644
> --- a/include/net/ip_vs.h
> +++ b/include/net/ip_vs.h
> @@ -18,6 +18,7 @@
>  #include <linux/workqueue.h>
>  
>  #include <linux/compiler.h>
> +#include <linux/jiffies.h>

	The patch looks good to me but you should not include
jiffies.h

> diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
> index 157f5c8361ab..175b842cdfec 100644
> --- a/net/netfilter/ipvs/ip_vs_sync.c
> +++ b/net/netfilter/ipvs/ip_vs_sync.c
> @@ -847,7 +847,7 @@ static void ip_vs_proc_conn(struct netns_ipvs *ipvs, struct ip_vs_conn_param *pa
>  			    unsigned int flags, unsigned int state,
>  			    unsigned int protocol, unsigned int type,
>  			    const union nf_inet_addr *daddr, __be16 dport,
> -			    unsigned long timeout, __u32 fwmark,
> +			    unsigned int timeout, __u32 fwmark,
>  			    struct ip_vs_sync_conn_options *opt)
>  {
>  	struct ip_vs_dest *dest;

	The above hunk does not apply cleanly due to a pending
work "ipvs: update flags and seqs in backup server under lock".
I'll let you know when you can post v2 after removing the jiffies.h
above and re-syncing after the above patch.

> @@ -939,9 +939,7 @@ static void ip_vs_proc_conn(struct netns_ipvs *ipvs, struct ip_vs_conn_param *pa
>  	 *  - No problem.
>  	 */
>  	if (timeout) {
> -		if (timeout > MAX_SCHEDULE_TIMEOUT / HZ)
> -			timeout = MAX_SCHEDULE_TIMEOUT / HZ;
> -		cp->timeout = timeout*HZ;
> +		cp->timeout = ip_vs_timeout_ticks(timeout);
>  	} else {
>  		struct ip_vs_proto_data *pd;
>  
> 
> base-commit: 734bef548a96e8774d6eef5dfa2026c6edc4e80b
> -- 
> 2.43.0

Regards

--
Julian Anastasov <ja@xxxxxx>