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

[Benjamin Marzinski <bmarzins@redhat.com>]

On Fri, Jul 03, 2026 at 01:07:21PM -0400, Yury Norov wrote:
> 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.

Err... I mean, an unsigned long.

> > 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?

Oops. I'll send a v2 patch where I clean up the commit message to
mention the correct argument type and don't accidentally turn my
reproducer commands into comments by copy-pasting the hash sign from the
shell prompt. :(

The reproducer is:
$ dmsetup create bigzero --table '0 8589934590 zero'
$ dmsetup create mymirror --table '0 8589934590 mirror core 2 2 nosync 2 /dev/mapper/bigzero 0 /dev/mapper/bigzero 0'

This will crash the machine without the fix.

>  
> > 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.

Yep. The code uses unsigned long. The commit message was a brain fart.
 
> Can you please share more details about your case? What does 'faking
> dm-mirror device' mean? Is this a real case in production environment?

I used dm-zero to make a virtual device that's just under 4TB, which I
used for both legs of the mirror. That is an easy way to tigger the
kernel panic, but you could hit this with any device larger than 2TB. At
these sizes, you would need a 2 sector region size to hit this. I hope
nobody does that. Picking a region size that is less than a page size
should probably become an error. LVM's default region size for mirror
devices is 4096 sectors. To see this issue with a region size like that,
you would need to be mirroring 4PB devices, and I assume that anyone
with massive devices would use a region size larger than the default. So
I doubt that anyone has hit this on a production system.

-Ben

> 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