Re: [PATCH] bitops: make the *_bit_le functions use unsigned long

[Yury Norov <ynorov@nvidia.com>]

Thanks for the patch!

On Thu, Jul 02, 2026 at 03:43:07PM -0400, Benjamin Marzinski wrote:
> The *_bit_le functions use a signed integer for the bit number.
> However, the *_bit functions can use an unsigned long. This causes
> problems if there is a large bitmap and a bit number > 0x80000000 is
> passed in. Since that is a negative int, it will get sign extended to a
> long when getting passed to the *_bit function, turning it into a huge
> bit number. This usually ends up with the memory address wrapping around
> and the function accessing memory before the start of the bitmap.
> 
> Avoid this by making the *_bit_le functions take an unsigned int.
> 
> This can be triggered by faking a huge dm-mirror device, which uses
> bitmaps to track the mirror regions:

Did you miss some part after the colon? Can you add a reproducer here?
 
> This will access memory before the start of the sync_bits bitmap, and
> likely hit the guard page of the previously allocated clean_bits bitmap.
> 
> I looked and didn't see any crazy code using the signed int to
> intentionally try and access bits before some address within the bitmap.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>

Taking the patch in bitmap-for-next.

I'm aware of the int vs unsigned int problem, and already aligned some
bitmaps code. The cpumasks and nodemasks are all signed, but that's
not a problem in there.

The 0x80000000-bit bitmap is 256MB, and I was not aware about bitmaps of
that size. If that starts showing up, switching to unsigned ints would
only double the capacity, and that may become not enough quite shortly.

Can you please share more details about your case? What does 'faking
dm-mirror device' mean? Is this a real case in production environment?

Thanks,
Yury

> ---
>  include/asm-generic/bitops/le.h | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/include/asm-generic/bitops/le.h b/include/asm-generic/bitops/le.h
> index d51beff60375..e3b0da9a25f1 100644
> --- a/include/asm-generic/bitops/le.h
> +++ b/include/asm-generic/bitops/le.h
> @@ -16,47 +16,47 @@
>  #endif
>  
>  
> -static inline int test_bit_le(int nr, const void *addr)
> +static inline int test_bit_le(unsigned long nr, const void *addr)
>  {
>  	return test_bit(nr ^ BITOP_LE_SWIZZLE, addr);
>  }
>  
> -static inline void set_bit_le(int nr, void *addr)
> +static inline void set_bit_le(unsigned long nr, void *addr)
>  {
>  	set_bit(nr ^ BITOP_LE_SWIZZLE, addr);
>  }
>  
> -static inline void clear_bit_le(int nr, void *addr)
> +static inline void clear_bit_le(unsigned long nr, void *addr)
>  {
>  	clear_bit(nr ^ BITOP_LE_SWIZZLE, addr);
>  }
>  
> -static inline void __set_bit_le(int nr, void *addr)
> +static inline void __set_bit_le(unsigned long nr, void *addr)
>  {
>  	__set_bit(nr ^ BITOP_LE_SWIZZLE, addr);
>  }
>  
> -static inline void __clear_bit_le(int nr, void *addr)
> +static inline void __clear_bit_le(unsigned long nr, void *addr)
>  {
>  	__clear_bit(nr ^ BITOP_LE_SWIZZLE, addr);
>  }
>  
> -static inline int test_and_set_bit_le(int nr, void *addr)
> +static inline int test_and_set_bit_le(unsigned long nr, void *addr)
>  {
>  	return test_and_set_bit(nr ^ BITOP_LE_SWIZZLE, addr);
>  }
>  
> -static inline int test_and_clear_bit_le(int nr, void *addr)
> +static inline int test_and_clear_bit_le(unsigned long nr, void *addr)
>  {
>  	return test_and_clear_bit(nr ^ BITOP_LE_SWIZZLE, addr);
>  }
>  
> -static inline int __test_and_set_bit_le(int nr, void *addr)
> +static inline int __test_and_set_bit_le(unsigned long nr, void *addr)
>  {
>  	return __test_and_set_bit(nr ^ BITOP_LE_SWIZZLE, addr);
>  }
>  
> -static inline int __test_and_clear_bit_le(int nr, void *addr)
> +static inline int __test_and_clear_bit_le(unsigned long nr, void *addr)
>  {
>  	return __test_and_clear_bit(nr ^ BITOP_LE_SWIZZLE, addr);
>  }
> -- 
> 2.53.0