From c0bf8816c9e2a4701c925d8b23abfda5990a0087 Mon Sep 17 00:00:00 2001 From: Andrew Cooper Date: Thu, 21 Nov 2024 14:00:43 +0000 Subject: [PATCH] xen/bitops: Fix break usage in for_each_set_bit() loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit for_each_set_bit()'s use of a double for loop had an accidental bug with a break in the inner loop leading to an infinite outer loop. Adjust for_each_set_bit() to avoid this behaviour, and add extend test_for_each_set_bit() with a test case for this. Fixes: ed26376f20bf ("xen/bitops: Introduce for_each_set_bit()") Signed-off-by: Andrew Cooper Reviewed-by: Frediano Ziglio Reviewed-by: Jan Beulich Reviewed-by: Roger Pau Monné --- xen/common/bitops.c | 17 ++++++++++++++++- xen/include/xen/bitops.h | 2 +- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/xen/common/bitops.c b/xen/common/bitops.c index 91ae961440af..bd17ddef5700 100644 --- a/xen/common/bitops.c +++ b/xen/common/bitops.c @@ -86,7 +86,7 @@ static void __init test_fls(void) static void __init test_for_each_set_bit(void) { - unsigned int ui, ui_res = 0; + unsigned int ui, ui_res = 0, tmp; unsigned long ul, ul_res = 0; uint64_t ull, ull_res = 0; @@ -110,6 +110,21 @@ static void __init test_for_each_set_bit(void) if ( ull != ull_res ) panic("for_each_set_bit(uint64) expected %#"PRIx64", got %#"PRIx64"\n", ull, ull_res); + + /* Check that we can break from the middle of the loop. */ + ui = HIDE(0x80001008U); + tmp = 0; + ui_res = 0; + for_each_set_bit ( i, ui ) + { + if ( tmp++ > 1 ) + break; + + ui_res |= 1U << i; + } + + if ( ui_res != 0x1008 ) + panic("for_each_set_bit(break) expected 0x1008, got %#x\n", ui_res); } static void __init test_multiple_bits_set(void) diff --git a/xen/include/xen/bitops.h b/xen/include/xen/bitops.h index 79615fb89d04..448b2d3e0937 100644 --- a/xen/include/xen/bitops.h +++ b/xen/include/xen/bitops.h @@ -299,7 +299,7 @@ static always_inline attr_const unsigned int fls64(uint64_t x) * A copy of @val is taken internally. */ #define for_each_set_bit(iter, val) \ - for ( typeof(val) __v = (val); __v; ) \ + for ( typeof(val) __v = (val); __v; __v = 0 ) \ for ( unsigned int (iter); \ __v && ((iter) = ffs_g(__v) - 1, true); \ __v &= __v - 1 )