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 Link: https://lore.kernel.org/r/4f420fda8690ac470be44b3da284b65501c77bb5.1793430892.git.x-professor@outlook.com Link: https://lore.kernel.org/r/5fa37fba-530c-2c5b-038e-9cb95e34023d@ssi.bg Signed-off-by: Yingjie Wang --- 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@outlook.com 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 #include +#include #include #include @@ -55,6 +56,18 @@ #define IP_VS_CONN_MAX (1 << 24) #endif +/* All timeouts up to 31 days */ +#define IP_VS_TIMEOUT_MAX_SECS (86400U * 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; +} + /* svc_table limits */ #define IP_VS_SVC_TAB_MIN_BITS 4 #define IP_VS_SVC_TAB_MAX_BITS 20 @@ -776,7 +789,7 @@ struct ip_vs_protocol { struct ip_vs_proto_data { struct ip_vs_proto_data *next; struct ip_vs_protocol *pp; - int *timeout_table; /* protocol timeout table */ + unsigned long *timeout_table; /* protocol timeout table */ atomic_t appcnt; /* counter of proto app incs. */ struct tcp_states_t *tcp_state_table; }; @@ -938,7 +951,7 @@ struct ip_vs_service { atomic_t refcnt; /* reference counter */ __be16 port; /* port number for the service */ unsigned int flags; /* service status flags */ - unsigned int timeout; /* persistent timeout in ticks */ + unsigned long timeout; /* persistent timeout in ticks */ __be32 netmask; /* grouping granularity, mask/plen */ struct netns_ipvs *ipvs; @@ -1844,7 +1857,8 @@ struct ip_vs_pe *__ip_vs_pe_getbyname(const char *pe_name); int ip_vs_protocol_init(void); void ip_vs_protocol_cleanup(void); void ip_vs_protocol_timeout_change(struct netns_ipvs *ipvs, int flags); -int *ip_vs_create_timeout_table(int *table, int size); +unsigned long *ip_vs_create_timeout_table(const unsigned long *table, + size_t size); void ip_vs_tcpudp_debug_packet(int af, struct ip_vs_protocol *pp, const struct sk_buff *skb, int offset, const char *msg); diff --git a/net/netfilter/ipvs/ip_vs_app.c b/net/netfilter/ipvs/ip_vs_app.c index 11cbdbaf561d..89be90a2c383 100644 --- a/net/netfilter/ipvs/ip_vs_app.c +++ b/net/netfilter/ipvs/ip_vs_app.c @@ -93,8 +93,7 @@ ip_vs_app_inc_new(struct netns_ipvs *ipvs, struct ip_vs_app *app, __u16 proto, if (app->timeouts) { inc->timeout_table = - ip_vs_create_timeout_table(app->timeouts, - app->timeouts_size); + kmemdup(app->timeouts, app->timeouts_size, GFP_KERNEL); if (!inc->timeout_table) { ret = -ENOMEM; goto out; diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c index ce9fe20d6106..5f19fd9545a6 100644 --- a/net/netfilter/ipvs/ip_vs_ctl.c +++ b/net/netfilter/ipvs/ip_vs_ctl.c @@ -1766,6 +1766,9 @@ ip_vs_add_service(struct netns_ipvs *ipvs, struct ip_vs_service_user_kern *u, int ret = 0; bool grow; + if (u->timeout > IP_VS_TIMEOUT_MAX_SECS) + return -EINVAL; + /* increase the module use count */ if (!ip_vs_use_count_inc()) return -ENOPROTOOPT; @@ -1864,7 +1867,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_timeout_ticks(u->timeout); svc->netmask = u->netmask; svc->ipvs = ipvs; @@ -1971,6 +1974,9 @@ ip_vs_edit_service(struct ip_vs_service *svc, struct ip_vs_service_user_kern *u) struct netns_ipvs *ipvs = svc->ipvs; int af_id = ip_vs_af_index(svc->af); + if (u->timeout > IP_VS_TIMEOUT_MAX_SECS) + return -EINVAL; + /* * Lookup the scheduler, by 'u->sched_name' */ @@ -2031,7 +2037,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_timeout_ticks(u->timeout); svc->netmask = u->netmask; old_pe = rcu_dereference_protected(svc->pe, 1); @@ -3064,9 +3070,8 @@ 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 %lu %08X\n", + svc->timeout, ntohl(svc->netmask)); else seq_putc(seq, '\n'); @@ -3365,36 +3370,38 @@ static int ip_vs_set_timeout(struct netns_ipvs *ipvs, struct ip_vs_timeout_user u->udp_timeout); #ifdef CONFIG_IP_VS_PROTO_TCP - if (u->tcp_timeout < 0 || u->tcp_timeout > (INT_MAX / HZ) || - u->tcp_fin_timeout < 0 || u->tcp_fin_timeout > (INT_MAX / HZ)) { + if (u->tcp_timeout < 0 || + u->tcp_timeout > IP_VS_TIMEOUT_MAX_SECS || + u->tcp_fin_timeout < 0 || + u->tcp_fin_timeout > IP_VS_TIMEOUT_MAX_SECS) { return -EINVAL; } #endif #ifdef CONFIG_IP_VS_PROTO_UDP - if (u->udp_timeout < 0 || u->udp_timeout > (INT_MAX / HZ)) + if (u->udp_timeout < 0 || u->udp_timeout > IP_VS_TIMEOUT_MAX_SECS) return -EINVAL; #endif #ifdef CONFIG_IP_VS_PROTO_TCP if (u->tcp_timeout) { pd = ip_vs_proto_data_get(ipvs, IPPROTO_TCP); - pd->timeout_table[IP_VS_TCP_S_ESTABLISHED] - = u->tcp_timeout * HZ; + pd->timeout_table[IP_VS_TCP_S_ESTABLISHED] = + ip_vs_timeout_ticks(u->tcp_timeout); } if (u->tcp_fin_timeout) { pd = ip_vs_proto_data_get(ipvs, IPPROTO_TCP); - pd->timeout_table[IP_VS_TCP_S_FIN_WAIT] - = u->tcp_fin_timeout * HZ; + pd->timeout_table[IP_VS_TCP_S_FIN_WAIT] = + ip_vs_timeout_ticks(u->tcp_fin_timeout); } #endif #ifdef CONFIG_IP_VS_PROTO_UDP if (u->udp_timeout) { pd = ip_vs_proto_data_get(ipvs, IPPROTO_UDP); - pd->timeout_table[IP_VS_UDP_S_NORMAL] - = u->udp_timeout * HZ; + pd->timeout_table[IP_VS_UDP_S_NORMAL] = + ip_vs_timeout_ticks(u->udp_timeout); } #endif return 0; diff --git a/net/netfilter/ipvs/ip_vs_proto.c b/net/netfilter/ipvs/ip_vs_proto.c index 0046aed1fb38..3796cd93c028 100644 --- a/net/netfilter/ipvs/ip_vs_proto.c +++ b/net/netfilter/ipvs/ip_vs_proto.c @@ -185,8 +185,8 @@ void ip_vs_protocol_timeout_change(struct netns_ipvs *ipvs, int flags) } -int * -ip_vs_create_timeout_table(int *table, int size) +unsigned long * +ip_vs_create_timeout_table(const unsigned long *table, size_t size) { return kmemdup(table, size, GFP_KERNEL); } diff --git a/net/netfilter/ipvs/ip_vs_proto_sctp.c b/net/netfilter/ipvs/ip_vs_proto_sctp.c index fb8af6b15a39..7e447b99d94c 100644 --- a/net/netfilter/ipvs/ip_vs_proto_sctp.c +++ b/net/netfilter/ipvs/ip_vs_proto_sctp.c @@ -327,7 +327,7 @@ static const __u8 sctp_states #define IP_VS_SCTP_MAX_RTO ((60 + 1) * HZ) /* Timeout table[state] */ -static const int sctp_timeouts[IP_VS_SCTP_S_LAST + 1] = { +static const unsigned long sctp_timeouts[IP_VS_SCTP_S_LAST + 1] = { [IP_VS_SCTP_S_NONE] = 2 * HZ, [IP_VS_SCTP_S_INIT1] = (0 + 3 + 1) * HZ, [IP_VS_SCTP_S_INIT] = IP_VS_SCTP_MAX_RTO, @@ -552,8 +552,8 @@ static int sctp_app_conn_bind(struct ip_vs_conn *cp) static int __ip_vs_sctp_init(struct netns_ipvs *ipvs, struct ip_vs_proto_data *pd) { ip_vs_init_hash_table(ipvs->sctp_apps, SCTP_APP_TAB_SIZE); - pd->timeout_table = ip_vs_create_timeout_table((int *)sctp_timeouts, - sizeof(sctp_timeouts)); + pd->timeout_table = + ip_vs_create_timeout_table(sctp_timeouts, sizeof(sctp_timeouts)); if (!pd->timeout_table) return -ENOMEM; return 0; diff --git a/net/netfilter/ipvs/ip_vs_proto_tcp.c b/net/netfilter/ipvs/ip_vs_proto_tcp.c index fec0e8b47b71..923405b65a33 100644 --- a/net/netfilter/ipvs/ip_vs_proto_tcp.c +++ b/net/netfilter/ipvs/ip_vs_proto_tcp.c @@ -326,7 +326,7 @@ static const int tcp_state_off[IP_VS_DIR_LAST] = { /* * Timeout table[state] */ -static const int tcp_timeouts[IP_VS_TCP_S_LAST+1] = { +static const unsigned long tcp_timeouts[IP_VS_TCP_S_LAST + 1] = { [IP_VS_TCP_S_NONE] = 2*HZ, [IP_VS_TCP_S_ESTABLISHED] = 15*60*HZ, [IP_VS_TCP_S_SYN_SENT] = 2*60*HZ, @@ -665,8 +665,8 @@ void ip_vs_tcp_conn_listen(struct ip_vs_conn *cp) static int __ip_vs_tcp_init(struct netns_ipvs *ipvs, struct ip_vs_proto_data *pd) { ip_vs_init_hash_table(ipvs->tcp_apps, TCP_APP_TAB_SIZE); - pd->timeout_table = ip_vs_create_timeout_table((int *)tcp_timeouts, - sizeof(tcp_timeouts)); + pd->timeout_table = + ip_vs_create_timeout_table(tcp_timeouts, sizeof(tcp_timeouts)); if (!pd->timeout_table) return -ENOMEM; pd->tcp_state_table = tcp_states; diff --git a/net/netfilter/ipvs/ip_vs_proto_udp.c b/net/netfilter/ipvs/ip_vs_proto_udp.c index f32785682402..b4de47f8273e 100644 --- a/net/netfilter/ipvs/ip_vs_proto_udp.c +++ b/net/netfilter/ipvs/ip_vs_proto_udp.c @@ -396,7 +396,7 @@ static int udp_app_conn_bind(struct ip_vs_conn *cp) } -static const int udp_timeouts[IP_VS_UDP_S_LAST+1] = { +static const unsigned long udp_timeouts[IP_VS_UDP_S_LAST + 1] = { [IP_VS_UDP_S_NORMAL] = 5*60*HZ, [IP_VS_UDP_S_LAST] = 2*HZ, }; @@ -432,8 +432,8 @@ udp_state_transition(struct ip_vs_conn *cp, int direction, static int __udp_init(struct netns_ipvs *ipvs, struct ip_vs_proto_data *pd) { ip_vs_init_hash_table(ipvs->udp_apps, UDP_APP_TAB_SIZE); - pd->timeout_table = ip_vs_create_timeout_table((int *)udp_timeouts, - sizeof(udp_timeouts)); + pd->timeout_table = + ip_vs_create_timeout_table(udp_timeouts, sizeof(udp_timeouts)); if (!pd->timeout_table) return -ENOMEM; return 0; 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; @@ -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