[PATCH 0/2] lib/bitmap: fix off-by-one in bitmap_to_arr64()

[PATCH 0/2] lib/bitmap: fix off-by-one in bitmap_to_arr64()

[Alexander Lobakin]

Fix tail clearing in bitmap_to_arr64() for 32-bit BEs and expand
the tests to cover the bug being fixed.

Alexander Lobakin (2):
  lib/bitmap: fix off-by-one in bitmap_to_arr64()
  lib/test_bitmap: test the tail after bitmap_to_arr64()

 lib/bitmap.c      | 2 +-
 lib/test_bitmap.c | 6 ++++++
 2 files changed, 7 insertions(+), 1 deletion(-)


base-commit: 32346491ddf24599decca06190ebca03ff9de7f8
-- 
2.36.1

[PATCH 1/2] lib/bitmap: fix off-by-one in bitmap_to_arr64()

[Alexander Lobakin]

GENMASK*() family takes the first and the last bits of the mask
*including* them. So, with the current code bitmap_to_arr64()
doesn't clear the tail properly:

nbits %  exp             mask                must be
1        GENMASK(1, 0)   0x3                 0x1
...
63       GENMASK(63, 0)  0xffffffffffffffff  0x7fffffffffffffff

This was found by making the function always available instead of
32-bit BE systems only (for reusing in some new functionality).
Turn the number of bits into the last bit set by subtracting 1.
@nbits is already checked to be positive beforehand.

Fixes: 0a97953fd221 ("lib: add bitmap_{from,to}_arr64")
Signed-off-by: Alexander Lobakin <alexandr.lobakin@intel.com>
---
 lib/bitmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/bitmap.c b/lib/bitmap.c
index b18e31ea6e66..e903e13c62e1 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -1564,7 +1564,7 @@ void bitmap_to_arr64(u64 *buf, const unsigned long *bitmap, unsigned int nbits)
 
 	/* Clear tail bits in the last element of array beyond nbits. */
 	if (nbits % 64)
-		buf[-1] &= GENMASK_ULL(nbits % 64, 0);
+		buf[-1] &= GENMASK_ULL((nbits - 1) % 64, 0);
 }
 EXPORT_SYMBOL(bitmap_to_arr64);
 #endif
-- 
2.36.1

[PATCH 2/2] lib/test_bitmap: test the tail after bitmap_to_arr64()

[Alexander Lobakin]

Currently, test_bitmap_arr64() only tests bitmap_to_arr64()'s sanity
by comparing the result of double-conversion (bm -> arr64 -> bm2)
with the input bitmap. However, this may be not enough when one side
hides bugs of the second one (e.g. tail clearing, which is being
performed by both).
Expand the tests and check the tail of the actual arr64 used as
a temporary buffer for double-converting.

Signed-off-by: Alexander Lobakin <alexandr.lobakin@intel.com>
---
 lib/test_bitmap.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lib/test_bitmap.c b/lib/test_bitmap.c
index d5923a640457..086b1d1db1ca 100644
--- a/lib/test_bitmap.c
+++ b/lib/test_bitmap.c
@@ -604,6 +604,12 @@ static void __init test_bitmap_arr64(void)
 			pr_err("bitmap_copy_arr64(nbits == %d:"
 				" tail is not safely cleared: %d\n", nbits, next_bit);
 
+		if ((nbits % 64) &&
+		    (arr[(nbits - 1) / 64] & ~GENMASK_ULL((nbits - 1) % 64, 0)))
+			pr_err("bitmap_to_arr64(nbits == %d): tail is not safely cleared: 0x%016llx (must be 0x%016llx)\n",
+			       nbits, arr[(nbits - 1) / 64],
+			       GENMASK_ULL((nbits - 1) % 64, 0));
+
 		if (nbits < EXP1_IN_BITS - 64)
 			expect_eq_uint(arr[DIV_ROUND_UP(nbits, 64)], 0xa5a5a5a5);
 	}
-- 
2.36.1

Re: [PATCH 0/2] lib/bitmap: fix off-by-one in bitmap_to_arr64()

[Andy Shevchenko]

On Mon, Jul 11, 2022 at 08:09:28PM +0200, Alexander Lobakin wrote:
> Fix tail clearing in bitmap_to_arr64() for 32-bit BEs and expand
> the tests to cover the bug being fixed.

Thanks,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> Alexander Lobakin (2):
>   lib/bitmap: fix off-by-one in bitmap_to_arr64()
>   lib/test_bitmap: test the tail after bitmap_to_arr64()
> 
>  lib/bitmap.c      | 2 +-
>  lib/test_bitmap.c | 6 ++++++
>  2 files changed, 7 insertions(+), 1 deletion(-)

-- 
With Best Regards,
Andy Shevchenko