[PATCH net] ipvs: fix shift-out-of-bounds in ip_vs_rht_desired_size

[PATCH net] ipvs: fix shift-out-of-bounds in ip_vs_rht_desired_size

[Julian Anastasov]

Calling roundup_pow_of_two() with 0 has undefined result:

UBSAN: shift-out-of-bounds in ./include/linux/log2.h:57:13
shift exponent 64 is too large for 64-bit type 'unsigned long'
CPU: 1 UID: 0 PID: 77 Comm: kworker/u8:4 Not tainted syzkaller #0 PREEMPT(full)
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 04/18/2026
Workqueue: events_unbound conn_resize_work_handler
Call Trace:
 <TASK>
 dump_stack_lvl+0xe8/0x150 lib/dump_stack.c:120
 ubsan_epilogue+0xa/0x30 lib/ubsan.c:233
 __ubsan_handle_shift_out_of_bounds+0x385/0x410 lib/ubsan.c:494
 __roundup_pow_of_two include/linux/log2.h:57 [inline]
 ip_vs_rht_desired_size+0x2cf/0x410 net/netfilter/ipvs/ip_vs_core.c:240
 ip_vs_conn_desired_size net/netfilter/ipvs/ip_vs_conn.c:765 [inline]
 conn_resize_work_handler+0x1b6/0x14c0 net/netfilter/ipvs/ip_vs_conn.c:822
 process_one_work kernel/workqueue.c:3302 [inline]
 process_scheduled_works+0xb5d/0x1860 kernel/workqueue.c:3385
 worker_thread+0xa53/0xfc0 kernel/workqueue.c:3466
 kthread+0x388/0x470 kernel/kthread.c:436
 ret_from_fork+0x514/0xb70 arch/x86/kernel/process.c:158
 ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
 </TASK>

Reported-by: syzbot+217f1db9c791e27fe54a@syzkaller.appspotmail.com
Fixes: b655388111cf ("ipvs: add resizable hash tables")
Signed-off-by: Julian Anastasov <ja@ssi.bg>
---
 net/netfilter/ipvs/ip_vs_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
index f5b7a2047291..ea450944465f 100644
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -237,7 +237,7 @@ int ip_vs_rht_desired_size(struct netns_ipvs *ipvs, struct ip_vs_rht *t, int n,
 {
 	if (!t)
 		return 1 << min_bits;
-	n = roundup_pow_of_two(n);
+	n = n ? roundup_pow_of_two(n) : 1;
 	if (lfactor < 0) {
 		int factor = min(-lfactor, max_bits);
 
-- 
2.53.0

Re: [PATCH net] ipvs: fix shift-out-of-bounds in ip_vs_rht_desired_size

[Simon Horman]

From: 'Simon Horman' <horms@kernel.org>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
ipvs: fix shift-out-of-bounds in ip_vs_rht_desired_size

This commit addresses a shift-out-of-bounds issue when roundup_pow_of_two()
is called with 0 in ip_vs_rht_desired_size. It updates the assignment to fall
back to 1 if n is 0.

> diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
> index f5b7a2047291..ea450944465f 100644
> --- a/net/netfilter/ipvs/ip_vs_core.c
> +++ b/net/netfilter/ipvs/ip_vs_core.c
> @@ -237,7 +237,7 @@ int ip_vs_rht_desired_size(struct netns_ipvs *ipvs, struct ip_vs_rht *t, int n,
>  {
>  	if (!t)
>  		return 1 << min_bits;
> -	n = roundup_pow_of_two(n);
> +	n = n ? roundup_pow_of_two(n) : 1;

The parameter n is a signed int. If n happens to be negative, it will evaluate
to true and be passed to roundup_pow_of_two().

Because roundup_pow_of_two() casts the input to unsigned long, a negative
value like -1 becomes ULONG_MAX. This results in a shift exponent of 64,
triggering a shift-out-of-bounds regression. Would it be safer to check
for n <= 0 instead?

Additionally, if n exceeds 1.07 billion (1 << 30) on systems with large
amounts of RAM, roundup_pow_of_two(n) evaluates to 1UL << 31. Since n is
a 32-bit signed int, assigning 1UL << 31 back to n overflows and results
in a negative number.

Since subsequent bounds checks use signed comparisons:

>  	if (lfactor < 0) {
>  		int factor = min(-lfactor, max_bits);
>  
>  		n = min(n, 1 << (max_bits - factor));

These checks will fail to constrain the size correctly.

[ ... ]

>  	/* Shrink but keep it n * 2 to prevent frequent resizing */
>  	return clamp(n << 1, 1 << min_bits, 1 << max_bits);
>  }

Eventually, the function returns clamp(n << 1, 1 << min_bits, 1 << max_bits).
Shifting a negative number causes undefined behavior, and the clamp operation
will force the hash table to its absolute minimum size.

Could this cause millions of connections to be placed into a minimally sized
hash table, causing severe collisions during RCU hash lookups?

Re: [PATCH net] ipvs: fix shift-out-of-bounds in ip_vs_rht_desired_size

[Julian Anastasov]

	Hello,

On Thu, 30 Apr 2026, Simon Horman wrote:

> From: 'Simon Horman' <horms@kernel.org>
> 
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> Full review at: https://sashiko.dev
> ---
> ipvs: fix shift-out-of-bounds in ip_vs_rht_desired_size
> 
> This commit addresses a shift-out-of-bounds issue when roundup_pow_of_two()
> is called with 0 in ip_vs_rht_desired_size. It updates the assignment to fall
> back to 1 if n is 0.
> 
> > diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
> > index f5b7a2047291..ea450944465f 100644
> > --- a/net/netfilter/ipvs/ip_vs_core.c
> > +++ b/net/netfilter/ipvs/ip_vs_core.c
> > @@ -237,7 +237,7 @@ int ip_vs_rht_desired_size(struct netns_ipvs *ipvs, struct ip_vs_rht *t, int n,
> >  {
> >  	if (!t)
> >  		return 1 << min_bits;
> > -	n = roundup_pow_of_two(n);
> > +	n = n ? roundup_pow_of_two(n) : 1;

	To overflow it on 64-bit we need 600GB+ memory, the
conns are ~300 bytes.

	In v2 I changed it to n > 0. To solve it completely
we may need to introduce some kind of "conn_max" sysctl,
now per-netns. It can help to limit the conns which are
created GFP_ATOMIC while for other structures we will switch
to GFP_KERNEL_ACCOUNT.

> The parameter n is a signed int. If n happens to be negative, it will evaluate
> to true and be passed to roundup_pow_of_two().
> 
> Because roundup_pow_of_two() casts the input to unsigned long, a negative
> value like -1 becomes ULONG_MAX. This results in a shift exponent of 64,
> triggering a shift-out-of-bounds regression. Would it be safer to check
> for n <= 0 instead?
> 
> Additionally, if n exceeds 1.07 billion (1 << 30) on systems with large
> amounts of RAM, roundup_pow_of_two(n) evaluates to 1UL << 31. Since n is
> a 32-bit signed int, assigning 1UL << 31 back to n overflows and results
> in a negative number.
> 
> Since subsequent bounds checks use signed comparisons:
> 
> >  	if (lfactor < 0) {
> >  		int factor = min(-lfactor, max_bits);
> >  
> >  		n = min(n, 1 << (max_bits - factor));
> 
> These checks will fail to constrain the size correctly.
> 
> [ ... ]
> 
> >  	/* Shrink but keep it n * 2 to prevent frequent resizing */
> >  	return clamp(n << 1, 1 << min_bits, 1 << max_bits);
> >  }
> 
> Eventually, the function returns clamp(n << 1, 1 << min_bits, 1 << max_bits).
> Shifting a negative number causes undefined behavior, and the clamp operation
> will force the hash table to its absolute minimum size.
> 
> Could this cause millions of connections to be placed into a minimally sized
> hash table, causing severe collisions during RCU hash lookups?

	All that is valid without conn/mem limits.

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: [PATCH net] ipvs: fix shift-out-of-bounds in ip_vs_rht_desired_size

[Simon Horman]

On Thu, Apr 30, 2026 at 04:10:16PM +0300, Julian Anastasov wrote:
> 
> 	Hello,
> 
> On Thu, 30 Apr 2026, Simon Horman wrote:
> 
> > From: 'Simon Horman' <horms@kernel.org>
> > 
> > This is an AI-generated review of your patch. The human sending this
> > email has considered the AI review valid, or at least plausible.
> > Full review at: https://sashiko.dev
> > ---
> > ipvs: fix shift-out-of-bounds in ip_vs_rht_desired_size
> > 
> > This commit addresses a shift-out-of-bounds issue when roundup_pow_of_two()
> > is called with 0 in ip_vs_rht_desired_size. It updates the assignment to fall
> > back to 1 if n is 0.
> > 
> > > diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
> > > index f5b7a2047291..ea450944465f 100644
> > > --- a/net/netfilter/ipvs/ip_vs_core.c
> > > +++ b/net/netfilter/ipvs/ip_vs_core.c
> > > @@ -237,7 +237,7 @@ int ip_vs_rht_desired_size(struct netns_ipvs *ipvs, struct ip_vs_rht *t, int n,
> > >  {
> > >  	if (!t)
> > >  		return 1 << min_bits;
> > > -	n = roundup_pow_of_two(n);
> > > +	n = n ? roundup_pow_of_two(n) : 1;
> 
> 	To overflow it on 64-bit we need 600GB+ memory, the
> conns are ~300 bytes.
> 
> 	In v2 I changed it to n > 0. To solve it completely
> we may need to introduce some kind of "conn_max" sysctl,
> now per-netns. It can help to limit the conns which are
> created GFP_ATOMIC while for other structures we will switch
> to GFP_KERNEL_ACCOUNT.

Thanks, I agree with this approach.

> 
> > The parameter n is a signed int. If n happens to be negative, it will evaluate
> > to true and be passed to roundup_pow_of_two().
> > 
> > Because roundup_pow_of_two() casts the input to unsigned long, a negative
> > value like -1 becomes ULONG_MAX. This results in a shift exponent of 64,
> > triggering a shift-out-of-bounds regression. Would it be safer to check
> > for n <= 0 instead?
> > 
> > Additionally, if n exceeds 1.07 billion (1 << 30) on systems with large
> > amounts of RAM, roundup_pow_of_two(n) evaluates to 1UL << 31. Since n is
> > a 32-bit signed int, assigning 1UL << 31 back to n overflows and results
> > in a negative number.
> > 
> > Since subsequent bounds checks use signed comparisons:
> > 
> > >  	if (lfactor < 0) {
> > >  		int factor = min(-lfactor, max_bits);
> > >  
> > >  		n = min(n, 1 << (max_bits - factor));
> > 
> > These checks will fail to constrain the size correctly.
> > 
> > [ ... ]
> > 
> > >  	/* Shrink but keep it n * 2 to prevent frequent resizing */
> > >  	return clamp(n << 1, 1 << min_bits, 1 << max_bits);
> > >  }
> > 
> > Eventually, the function returns clamp(n << 1, 1 << min_bits, 1 << max_bits).
> > Shifting a negative number causes undefined behavior, and the clamp operation
> > will force the hash table to its absolute minimum size.
> > 
> > Could this cause millions of connections to be placed into a minimally sized
> > hash table, causing severe collisions during RCU hash lookups?
> 
> 	All that is valid without conn/mem limits.

Sorry, my previous email was supposed to come with some commentary
that I think that this is a pre-existing issue that can be treated
separately from this patch.