[PATCH] badblocks: fix infinite loop due to incorrect rounding and overflow

[PATCH] badblocks: fix infinite loop due to incorrect rounding and overflow

[Ramesh Adhikari]

The roundup() and rounddown() macros return the rounded value but
do not modify the input in place. In _badblocks_set(), _badblocks_clear(),
and badblocks_check(), the return values were being discarded, causing
s and target/next to remain unrounded. This resulted in sectors
being calculated from unrounded values, which could lead to sectors
being way too large (or zero), causing infinite loops in the
re_insert/re_clear/re_check loops.

Additionally, add integer overflow checks (s > ULLONG_MAX - sectors)
before the s + sectors calculation in all three functions to prevent
overflow-related issues. Also add early return when sectors becomes
zero after rounding in badblocks_check().

Root cause: When s and sectors have specific values (e.g., from
syzkaller fuzzing via nvdimm ioctl), the unrounded values cause
sectors to be incorrectly calculated. In _badblocks_clear(), this
could result in needing 2^46 iterations to process 2^55 sectors,
triggering RCU stall warnings and effectively hanging the kernel.

Fix by properly capturing the return values from roundup() and
rounddown(), adding overflow checks before sector arithmetic, and
handling the zero-sectors case in badblocks_check().

Signed-off-by: Ramesh Adhikari <adhikari.resume@gmail.com>
---
 block/badblocks.c | 43 ++++++++++++++++++++++++++++++++++---------
 1 file changed, 34 insertions(+), 9 deletions(-)

diff --git a/block/badblocks.c b/block/badblocks.c
index ece64e76fe8..a5ffae65a05 100644
--- a/block/badblocks.c
+++ b/block/badblocks.c
@@ -855,13 +855,21 @@ static bool _badblocks_set(struct badblocks *bb, sector_t s, sector_t sectors,
 
 	if (bb->shift) {
 		/* round the start down, and the end up */
+		if (s > ULLONG_MAX - sectors)
+			return false;
 		sector_t next = s + sectors;
 
-		rounddown(s, 1 << bb->shift);
-		roundup(next, 1 << bb->shift);
-		sectors = next - s;
+		s = rounddown(s, 1 << bb->shift);
+		next = roundup(next, 1 << bb->shift);
+		if (next < s)
+			sectors = 0;
+		else
+			sectors = next - s;
 	}
 
+	if (sectors == 0)
+		return false;
+
 	write_seqlock_irqsave(&bb->lock, flags);
 
 	bad.ack = acknowledged;
@@ -1070,12 +1078,20 @@ static bool _badblocks_clear(struct badblocks *bb, sector_t s, sector_t sectors)
 		 * However it is better the think a block is bad when it
 		 * isn't than to think a block is not bad when it is.
 		 */
+		if (s > ULLONG_MAX - sectors)
+			return false;
 		target = s + sectors;
-		roundup(s, 1 << bb->shift);
-		rounddown(target, 1 << bb->shift);
-		sectors = target - s;
+		s = roundup(s, 1 << bb->shift);
+		target = rounddown(target, 1 << bb->shift);
+		if (target < s)
+			sectors = 0;
+		else
+			sectors = target - s;
 	}
 
+	if (sectors == 0)
+		return false;
+
 	write_seqlock_irq(&bb->lock);
 
 	bad.ack = true;
@@ -1305,11 +1321,20 @@ int badblocks_check(struct badblocks *bb, sector_t s, sector_t sectors,
 
 	if (bb->shift > 0) {
 		/* round the start down, and the end up */
+		if (s > ULLONG_MAX - sectors) {
+			return -EINVAL;
+		}
 		sector_t target = s + sectors;
 
-		rounddown(s, 1 << bb->shift);
-		roundup(target, 1 << bb->shift);
-		sectors = target - s;
+		s = rounddown(s, 1 << bb->shift);
+		target = roundup(target, 1 << bb->shift);
+		if (target < s)
+			sectors = 0;
+		else
+			sectors = target - s;
+
+		if (sectors == 0)
+			return 0;
 	}
 
 retry:
-- 
2.43.0

Re: [PATCH] badblocks: fix infinite loop due to incorrect rounding and overflow

[Greg KH]

On Mon, Apr 27, 2026 at 08:40:48PM +0530, Ramesh Adhikari wrote:
> The roundup() and rounddown() macros return the rounded value but
> do not modify the input in place. In _badblocks_set(), _badblocks_clear(),
> and badblocks_check(), the return values were being discarded, causing
> s and target/next to remain unrounded. This resulted in sectors
> being calculated from unrounded values, which could lead to sectors
> being way too large (or zero), causing infinite loops in the
> re_insert/re_clear/re_check loops.
> 
> Additionally, add integer overflow checks (s > ULLONG_MAX - sectors)
> before the s + sectors calculation in all three functions to prevent
> overflow-related issues. Also add early return when sectors becomes
> zero after rounding in badblocks_check().
> 
> Root cause: When s and sectors have specific values (e.g., from
> syzkaller fuzzing via nvdimm ioctl), the unrounded values cause
> sectors to be incorrectly calculated. In _badblocks_clear(), this
> could result in needing 2^46 iterations to process 2^55 sectors,
> triggering RCU stall warnings and effectively hanging the kernel.
> 
> Fix by properly capturing the return values from roundup() and
> rounddown(), adding overflow checks before sector arithmetic, and
> handling the zero-sectors case in badblocks_check().
> 
> Signed-off-by: Ramesh Adhikari <adhikari.resume@gmail.com>
> ---
>  block/badblocks.c | 43 ++++++++++++++++++++++++++++++++++---------
>  1 file changed, 34 insertions(+), 9 deletions(-)
> 
> diff --git a/block/badblocks.c b/block/badblocks.c
> index ece64e76fe8..a5ffae65a05 100644
> --- a/block/badblocks.c
> +++ b/block/badblocks.c
> @@ -855,13 +855,21 @@ static bool _badblocks_set(struct badblocks *bb, sector_t s, sector_t sectors,
>  
>  	if (bb->shift) {
>  		/* round the start down, and the end up */
> +		if (s > ULLONG_MAX - sectors)
> +			return false;
>  		sector_t next = s + sectors;
>  
> -		rounddown(s, 1 << bb->shift);
> -		roundup(next, 1 << bb->shift);
> -		sectors = next - s;
> +		s = rounddown(s, 1 << bb->shift);
> +		next = roundup(next, 1 << bb->shift);
> +		if (next < s)
> +			sectors = 0;
> +		else
> +			sectors = next - s;
>  	}
>  
> +	if (sectors == 0)
> +		return false;
> +
>  	write_seqlock_irqsave(&bb->lock, flags);
>  
>  	bad.ack = acknowledged;
> @@ -1070,12 +1078,20 @@ static bool _badblocks_clear(struct badblocks *bb, sector_t s, sector_t sectors)
>  		 * However it is better the think a block is bad when it
>  		 * isn't than to think a block is not bad when it is.
>  		 */
> +		if (s > ULLONG_MAX - sectors)
> +			return false;
>  		target = s + sectors;
> -		roundup(s, 1 << bb->shift);
> -		rounddown(target, 1 << bb->shift);
> -		sectors = target - s;
> +		s = roundup(s, 1 << bb->shift);
> +		target = rounddown(target, 1 << bb->shift);
> +		if (target < s)
> +			sectors = 0;
> +		else
> +			sectors = target - s;
>  	}
>  
> +	if (sectors == 0)
> +		return false;
> +
>  	write_seqlock_irq(&bb->lock);
>  
>  	bad.ack = true;
> @@ -1305,11 +1321,20 @@ int badblocks_check(struct badblocks *bb, sector_t s, sector_t sectors,
>  
>  	if (bb->shift > 0) {
>  		/* round the start down, and the end up */
> +		if (s > ULLONG_MAX - sectors) {
> +			return -EINVAL;
> +		}
>  		sector_t target = s + sectors;
>  
> -		rounddown(s, 1 << bb->shift);
> -		roundup(target, 1 << bb->shift);
> -		sectors = target - s;
> +		s = rounddown(s, 1 << bb->shift);
> +		target = roundup(target, 1 << bb->shift);
> +		if (target < s)
> +			sectors = 0;
> +		else
> +			sectors = target - s;
> +
> +		if (sectors == 0)
> +			return 0;
>  	}
>  
>  retry:
> -- 
> 2.43.0
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- This looks like a new version of a previously submitted patch, but you
  did not list below the --- line any changes from the previous version.
  Please read the section entitled "The canonical patch format" in the
  kernel file, Documentation/process/submitting-patches.rst for what
  needs to be done here to properly describe this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

Re: [PATCH] badblocks: fix infinite loop due to incorrect rounding and overflow

[kernel test robot]

Hi Ramesh,

kernel test robot noticed the following build errors:

[auto build test ERROR on axboe/for-next]
[also build test ERROR on linus/master v7.1-rc1 next-20260429]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ramesh-Adhikari/badblocks-fix-infinite-loop-due-to-incorrect-rounding-and-overflow/20260429-044425
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux.git for-next
patch link:    https://lore.kernel.org/r/20260427151048.756072-1-adhikari.resume%40gmail.com
patch subject: [PATCH] badblocks: fix infinite loop due to incorrect rounding and overflow
config: arm-randconfig-002-20260430 (https://download.01.org/0day-ci/archive/20260430/202604300729.gKrAMFdE-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 8.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260430/202604300729.gKrAMFdE-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202604300729.gKrAMFdE-lkp@intel.com/

All errors (new ones prefixed by >>):

   arm-linux-gnueabi-ld: block/badblocks.o: in function `badblocks_clear':
>> badblocks.c:(.text+0x1d18): undefined reference to `__aeabi_uldivmod'
>> arm-linux-gnueabi-ld: badblocks.c:(.text+0x1d54): undefined reference to `__aeabi_uldivmod'
   arm-linux-gnueabi-ld: block/badblocks.o: in function `_badblocks_set':
   badblocks.c:(.text+0x2c90): undefined reference to `__aeabi_uldivmod'
   arm-linux-gnueabi-ld: badblocks.c:(.text+0x2ce0): undefined reference to `__aeabi_uldivmod'
   arm-linux-gnueabi-ld: block/badblocks.o: in function `badblocks_check':
   badblocks.c:(.text+0x3fb8): undefined reference to `__aeabi_uldivmod'
   arm-linux-gnueabi-ld: block/badblocks.o:badblocks.c:(.text+0x4018): more undefined references to `__aeabi_uldivmod' follow

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] badblocks: fix infinite loop due to incorrect rounding and overflow

[kernel test robot]

Hi Ramesh,

kernel test robot noticed the following build errors:

[auto build test ERROR on axboe/for-next]
[also build test ERROR on linus/master v7.1-rc1 next-20260429]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ramesh-Adhikari/badblocks-fix-infinite-loop-due-to-incorrect-rounding-and-overflow/20260429-044425
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux.git for-next
patch link:    https://lore.kernel.org/r/20260427151048.756072-1-adhikari.resume%40gmail.com
patch subject: [PATCH] badblocks: fix infinite loop due to incorrect rounding and overflow
config: i386-allnoconfig (https://download.01.org/0day-ci/archive/20260430/202604301231.IpPh4AiH-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260430/202604301231.IpPh4AiH-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202604301231.IpPh4AiH-lkp@intel.com/

All errors (new ones prefixed by >>):

   ld: block/badblocks.o: in function `badblocks_check':
   badblocks.c:(.text+0x10af): undefined reference to `__umoddi3'
>> ld: badblocks.c:(.text+0x1109): undefined reference to `__umoddi3'
   ld: block/badblocks.o: in function `_badblocks_set':
   badblocks.c:(.text+0x12b0): undefined reference to `__umoddi3'
   ld: badblocks.c:(.text+0x12f4): undefined reference to `__umoddi3'
   ld: block/badblocks.o: in function `_badblocks_clear':
   badblocks.c:(.text+0x1bfc): undefined reference to `__umoddi3'
   ld: block/badblocks.o:badblocks.c:(.text+0x1c1c): more undefined references to `__umoddi3' follow

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH v5] badblocks: fix infinite loop due to incorrect rounding and overflow

[Ramesh Adhikari]

The roundup() and rounddown() macros return the rounded value but
do not modify their input in place. In _badblocks_set(),
_badblocks_clear(), and badblocks_check(), the return values were
being discarded, so s and target/next remained unrounded. Sectors
were then calculated from these unrounded values, which could make
sectors way too large (or zero), causing infinite loops in the
re_insert/re_clear/re_check loops.

This was confirmed with local syzkaller fuzzing against the nvdimm
ioctl path (ND_IOCTL_CLEAR_ERROR -> nvdimm_clear_badblocks_region()
-> badblocks_clear()), which reliably produces RCU stalls with the
looping task caught mid-loop:

  rcu: INFO: rcu_preempt detected stalls on CPUs/tasks:
  rcu: rcu_preempt kthread starved for 21001 jiffies! g40229 f0x0 RCU_GP_WAIT_FQS(5)
  ...
  Call Trace:
   badblocks_clear+0x259/0xb10
   nvdimm_clear_badblocks_region+0x165/0x1e0 [libnvdimm]
   device_for_each_child+0x11e/0x1a0
   nd_ioctl+0x1413/0x1750 [libnvdimm]
   __x64_sys_ioctl+0x18e/0x210
   do_syscall_64+0x102/0x5a0

Fix this by properly capturing the return values of round_up()/
round_down() (the power-of-two variants, since 1 << bb->shift is
always a power of two -- roundup()/rounddown() use a division/
modulo internally, which is undesirable here). Also add overflow
checks (s > ULLONG_MAX - sectors) before the s + sectors addition
in all three functions, and handle the case where sectors becomes
zero after rounding.

The overflow check is done unconditionally, before the bb->shift
rounding block, rather than inside it. bb->shift == 0 is not just
an initial state -- __badblocks_init() sets it to 0 by default, and
drivers/md/md.c explicitly sets rdev->badblocks.shift back to 0 in
several paths -- so s + sectors needs the same overflow guard
whether or not rounding happens.

Signed-off-by: Ramesh Adhikari <adhikari.resume@gmail.com>
---
v1-v3 chased individual len==0 symptoms in _badblocks_clear()/
_badblocks_check() one call site at a time. Jens pointed out that
approach wasn't finding the actual bug, just papering over spots as
they were noticed. v4 was a full rewrite around the real root cause
(roundup()/rounddown() discarding their return values), covering all
three functions with proper overflow handling.

Changes in v5:
 - Switch from roundup()/rounddown() (which use division/modulo on
   sector_t, a u64) to round_up()/round_down() (bitmask-based, since
   1 << bb->shift is always a power of two). Fixes the v4 build
   failure kernel test robot reported on 32-bit (undefined reference
   to __aeabi_uldivmod on ARM, __umoddi3 on i386).
 - Move the s > ULLONG_MAX - sectors overflow check so it runs
   unconditionally in all three functions, instead of only inside
   the `if (bb->shift)` block. bb->shift == 0 is a real, common
   state (default init value, and explicitly set by drivers/md/md.c
   in several paths), so the overflow guard needs to apply there
   too, not just when rounding is active.
 - Build-tested locally (x86_64) and confirmed no residual div/mod
   symbols in the object file.

Link to v4: https://lore.kernel.org/r/20260427151048.756072-1-adhikari.resume@gmail.com

 block/badblocks.c | 40 ++++++++++++++++++++++++++++++++--------
 1 file changed, 32 insertions(+), 8 deletions(-)

diff --git a/block/badblocks.c b/block/badblocks.c
index ece64e76fe8..e8484912532 100644
--- a/block/badblocks.c
+++ b/block/badblocks.c
@@ -853,15 +853,21 @@ static bool _badblocks_set(struct badblocks *bb, sector_t s, sector_t sectors,
 		/* Invalid sectors number */
 		return false;
 
+	if (s > ULLONG_MAX - sectors)
+		return false;
+
 	if (bb->shift) {
 		/* round the start down, and the end up */
 		sector_t next = s + sectors;
 
-		rounddown(s, 1 << bb->shift);
-		roundup(next, 1 << bb->shift);
+		s = round_down(s, 1 << bb->shift);
+		next = round_up(next, 1 << bb->shift);
 		sectors = next - s;
 	}
 
+	if (sectors == 0)
+		return false;
+
 	write_seqlock_irqsave(&bb->lock, flags);
 
 	bad.ack = acknowledged;
@@ -1061,6 +1067,9 @@ static bool _badblocks_clear(struct badblocks *bb, sector_t s, sector_t sectors)
 		/* Invalid sectors number */
 		return false;
 
+	if (s > ULLONG_MAX - sectors)
+		return false;
+
 	if (bb->shift) {
 		sector_t target;
 
@@ -1071,11 +1080,17 @@ static bool _badblocks_clear(struct badblocks *bb, sector_t s, sector_t sectors)
 		 * isn't than to think a block is not bad when it is.
 		 */
 		target = s + sectors;
-		roundup(s, 1 << bb->shift);
-		rounddown(target, 1 << bb->shift);
-		sectors = target - s;
+		s = round_up(s, 1 << bb->shift);
+		target = round_down(target, 1 << bb->shift);
+		if (target < s)
+			sectors = 0;
+		else
+			sectors = target - s;
 	}
 
+	if (sectors == 0)
+		return false;
+
 	write_seqlock_irq(&bb->lock);
 
 	bad.ack = true;
@@ -1303,13 +1318,22 @@ int badblocks_check(struct badblocks *bb, sector_t s, sector_t sectors,
 
 	WARN_ON(bb->shift < 0 || sectors == 0);
 
+	if (s > ULLONG_MAX - sectors)
+		return -EINVAL;
+
 	if (bb->shift > 0) {
 		/* round the start down, and the end up */
 		sector_t target = s + sectors;
 
-		rounddown(s, 1 << bb->shift);
-		roundup(target, 1 << bb->shift);
-		sectors = target - s;
+		s = round_down(s, 1 << bb->shift);
+		target = round_up(target, 1 << bb->shift);
+		if (target < s)
+			sectors = 0;
+		else
+			sectors = target - s;
+
+		if (sectors == 0)
+			return 0;
 	}
 
 retry:
-- 
2.43.0

Re: [PATCH v5] badblocks: fix infinite loop due to incorrect rounding and overflow

[Coly Li]

On Sat, Jul 04, 2026 at 10:43:21PM +0800, Ramesh Adhikari wrote:
> The roundup() and rounddown() macros return the rounded value but
> do not modify their input in place. In _badblocks_set(),
> _badblocks_clear(), and badblocks_check(), the return values were
> being discarded, so s and target/next remained unrounded. Sectors
> were then calculated from these unrounded values, which could make
> sectors way too large (or zero), causing infinite loops in the
> re_insert/re_clear/re_check loops.
> 
> This was confirmed with local syzkaller fuzzing against the nvdimm
> ioctl path (ND_IOCTL_CLEAR_ERROR -> nvdimm_clear_badblocks_region()
> -> badblocks_clear()), which reliably produces RCU stalls with the
> looping task caught mid-loop:
> 
>   rcu: INFO: rcu_preempt detected stalls on CPUs/tasks:
>   rcu: rcu_preempt kthread starved for 21001 jiffies! g40229 f0x0 RCU_GP_WAIT_FQS(5)
>   ...
>   Call Trace:
>    badblocks_clear+0x259/0xb10
>    nvdimm_clear_badblocks_region+0x165/0x1e0 [libnvdimm]
>    device_for_each_child+0x11e/0x1a0
>    nd_ioctl+0x1413/0x1750 [libnvdimm]
>    __x64_sys_ioctl+0x18e/0x210
>    do_syscall_64+0x102/0x5a0
> 
> Fix this by properly capturing the return values of round_up()/
> round_down() (the power-of-two variants, since 1 << bb->shift is
> always a power of two -- roundup()/rounddown() use a division/
> modulo internally, which is undesirable here). Also add overflow
> checks (s > ULLONG_MAX - sectors) before the s + sectors addition
> in all three functions, and handle the case where sectors becomes
> zero after rounding.
> 
> The overflow check is done unconditionally, before the bb->shift
> rounding block, rather than inside it. bb->shift == 0 is not just
> an initial state -- __badblocks_init() sets it to 0 by default, and
> drivers/md/md.c explicitly sets rdev->badblocks.shift back to 0 in
> several paths -- so s + sectors needs the same overflow guard
> whether or not rounding happens.
> 
> Signed-off-by: Ramesh Adhikari <adhikari.resume@gmail.com>
> ---
> v1-v3 chased individual len==0 symptoms in _badblocks_clear()/
> _badblocks_check() one call site at a time. Jens pointed out that
> approach wasn't finding the actual bug, just papering over spots as
> they were noticed. v4 was a full rewrite around the real root cause
> (roundup()/rounddown() discarding their return values), covering all
> three functions with proper overflow handling.
> 
> Changes in v5:
>  - Switch from roundup()/rounddown() (which use division/modulo on
>    sector_t, a u64) to round_up()/round_down() (bitmask-based, since
>    1 << bb->shift is always a power of two). Fixes the v4 build
>    failure kernel test robot reported on 32-bit (undefined reference
>    to __aeabi_uldivmod on ARM, __umoddi3 on i386).
>  - Move the s > ULLONG_MAX - sectors overflow check so it runs
>    unconditionally in all three functions, instead of only inside
>    the `if (bb->shift)` block. bb->shift == 0 is a real, common
>    state (default init value, and explicitly set by drivers/md/md.c
>    in several paths), so the overflow guard needs to apply there
>    too, not just when rounding is active.
>  - Build-tested locally (x86_64) and confirmed no residual div/mod
>    symbols in the object file.
> 
> Link to v4: https://lore.kernel.org/r/20260427151048.756072-1-adhikari.resume@gmail.com
>

Hi Ramesh,

Thanks for the fix up.
 
>  block/badblocks.c | 40 ++++++++++++++++++++++++++++++++--------
>  1 file changed, 32 insertions(+), 8 deletions(-)
> 
> diff --git a/block/badblocks.c b/block/badblocks.c
> index ece64e76fe8..e8484912532 100644
> --- a/block/badblocks.c
> +++ b/block/badblocks.c
> @@ -853,15 +853,21 @@ static bool _badblocks_set(struct badblocks *bb, sector_t s, sector_t sectors,
>  		/* Invalid sectors number */
>  		return false;
>  
> +	if (s > ULLONG_MAX - sectors)
> +		return false;
> +

The original idea was to let caller to handle such overflow stuffs.
If you want to handle these things here, the above lines are not
enough indeed. You need to considering more conditions, I used to
unlike such codes, but now realize maybe such checks should be
added.

Here are some items in my brain,
- s + sectors overflow
- too big bb->shift
- 1 << bb->shift overflow
- the rounddown result even smaller then start s for large bb->shift


>  	if (bb->shift) {
>  		/* round the start down, and the end up */
>  		sector_t next = s + sectors;
>  
> -		rounddown(s, 1 << bb->shift);
> -		roundup(next, 1 << bb->shift);
> +		s = round_down(s, 1 << bb->shift);
> +		next = round_up(next, 1 << bb->shift);
>  		sectors = next - s;

I suggest to divide the patch into two. The first one just contains
the above round_down and round_up fix. Another one check the rested
value ranges.

Also add Fixes tag to my origin badblocks rewrite commit, and add
a CC line to stable@vger.kernel.org

Thanks.

Coly Li


>  	}
>  
> +	if (sectors == 0)
> +		return false;
> +
>  	write_seqlock_irqsave(&bb->lock, flags);
>  
>  	bad.ack = acknowledged;
> @@ -1061,6 +1067,9 @@ static bool _badblocks_clear(struct badblocks *bb, sector_t s, sector_t sectors)
>  		/* Invalid sectors number */
>  		return false;
>  
> +	if (s > ULLONG_MAX - sectors)
> +		return false;
> +
>  	if (bb->shift) {
>  		sector_t target;
>  
> @@ -1071,11 +1080,17 @@ static bool _badblocks_clear(struct badblocks *bb, sector_t s, sector_t sectors)
>  		 * isn't than to think a block is not bad when it is.
>  		 */
>  		target = s + sectors;
> -		roundup(s, 1 << bb->shift);
> -		rounddown(target, 1 << bb->shift);
> -		sectors = target - s;
> +		s = round_up(s, 1 << bb->shift);
> +		target = round_down(target, 1 << bb->shift);
> +		if (target < s)
> +			sectors = 0;
> +		else
> +			sectors = target - s;
>  	}
>  
> +	if (sectors == 0)
> +		return false;
> +
>  	write_seqlock_irq(&bb->lock);
>  
>  	bad.ack = true;
> @@ -1303,13 +1318,22 @@ int badblocks_check(struct badblocks *bb, sector_t s, sector_t sectors,
>  
>  	WARN_ON(bb->shift < 0 || sectors == 0);
>  
> +	if (s > ULLONG_MAX - sectors)
> +		return -EINVAL;
> +
>  	if (bb->shift > 0) {
>  		/* round the start down, and the end up */
>  		sector_t target = s + sectors;
>  
> -		rounddown(s, 1 << bb->shift);
> -		roundup(target, 1 << bb->shift);
> -		sectors = target - s;
> +		s = round_down(s, 1 << bb->shift);
> +		target = round_up(target, 1 << bb->shift);
> +		if (target < s)
> +			sectors = 0;
> +		else
> +			sectors = target - s;
> +
> +		if (sectors == 0)
> +			return 0;
>  	}
>  
>  retry:
> -- 
> 2.43.0
> 

[PATCH v6 0/2] badblocks: fix infinite loop and validate sector range/shift

[Ramesh Adhikari]

This replaces the single-patch v5 with a two-patch series, per Coly's
review.

v1-v4 chased symptoms of the same underlying bug (an RCU stall found
by syzkaller through the nvdimm ioctl path, in _badblocks_check() /
badblocks_check() looping with a non-advancing range) before landing
on the root cause in v4: rounddown()/roundup() don't modify their
argument in place, so 's'/'next'/'target' were never actually
rounded.

v5 folded the round_down()/round_up() fix together with overflow and
zero-length guards into one patch. Coly's review on v5 pointed out
that:

  - the overflow check that was there wasn't sufficient on its own
    (only one of several range-validity conditions), and
  - the round fix and the validation should be separate patches,
    since they're independently useful and one is safe to backport
    on its own.

This series:

  1/2 is exactly the round_down()/round_up() fix, nothing else. This
      is what actually stops the infinite loop and also fixes the
      32-bit build breakage kernel test robot reported on v1
      (rounddown()/roundup() do 64-bit division/modulo on sector_t,
      requiring libgcc helpers not linked into the kernel).

  2/2 adds the range/shift validation Coly asked for: s+sectors
      overflow, bb->shift too large to shift a sector_t by (bb->shift
      is populated in drivers/md/md.c straight from an unvalidated
      on-disk superblock byte), and detecting when round_up()/
      round_down() themselves wrap near ULLONG_MAX.

Both are tagged Fixes: aa511ff8218b ("badblocks: switch to the
improved badblock handling code") and Cc: stable, since that's the
commit that introduced this code path.

Ramesh Adhikari (2):
  badblocks: fix in-place round_up/round_down usage bug
  badblocks: validate sector range and shift before rounding

 block/badblocks.c | 52 ++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 45 insertions(+), 7 deletions(-)

--
2.43.0

[PATCH v6 1/2] badblocks: fix in-place round_up/round_down usage bug

[Ramesh Adhikari]

rounddown() and roundup() do not modify their first argument in
place; they return the rounded value. _badblocks_set(),
_badblocks_clear() and badblocks_check() were calling them as
bare statements and discarding the result, so 's' (and 'next'/
'target') were never actually rounded. Depending on the caller's
alignment this can leave 'sectors' unchanged or, in the reported
case, produce a range whose end never advances, causing
_badblocks_check()/badblocks_check() to loop with a non-advancing
cursor and stall the CPU (RCU stall) when called through the
nvdimm ioctl path via nvdimm_clear_badblocks_region().

rounddown()/roundup() also do division/modulo on the sector_t
(u64) operand, which requires libgcc helpers (__aeabi_uldivmod,
__umoddi3) that are not linked into the kernel on 32-bit builds,
breaking the build on arm/i386 (reported by kernel test robot).

Switch to round_down()/round_up() (include/linux/math.h), which
are mask-based, assign their result back to the variable being
rounded, and require no 64-bit division, fixing both the
non-rounding bug and the 32-bit build breakage.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202604301231.IpPh4AiH-lkp@intel.com/
Fixes: aa511ff8218b ("badblocks: switch to the improved badblock handling code")
Cc: stable@vger.kernel.org
Signed-off-by: Ramesh Adhikari <adhikari.resume@gmail.com>
---
 block/badblocks.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/block/badblocks.c b/block/badblocks.c
index ece64e76fe8..1f786b193fb 100644
--- a/block/badblocks.c
+++ b/block/badblocks.c
@@ -857,8 +857,8 @@ static bool _badblocks_set(struct badblocks *bb, sector_t s, sector_t sectors,
 		/* round the start down, and the end up */
 		sector_t next = s + sectors;
 
-		rounddown(s, 1 << bb->shift);
-		roundup(next, 1 << bb->shift);
+		s = round_down(s, 1 << bb->shift);
+		next = round_up(next, 1 << bb->shift);
 		sectors = next - s;
 	}
 
@@ -1071,8 +1071,8 @@ static bool _badblocks_clear(struct badblocks *bb, sector_t s, sector_t sectors)
 		 * isn't than to think a block is not bad when it is.
 		 */
 		target = s + sectors;
-		roundup(s, 1 << bb->shift);
-		rounddown(target, 1 << bb->shift);
+		s = round_up(s, 1 << bb->shift);
+		target = round_down(target, 1 << bb->shift);
 		sectors = target - s;
 	}
 
@@ -1307,8 +1307,8 @@ int badblocks_check(struct badblocks *bb, sector_t s, sector_t sectors,
 		/* round the start down, and the end up */
 		sector_t target = s + sectors;
 
-		rounddown(s, 1 << bb->shift);
-		roundup(target, 1 << bb->shift);
+		s = round_down(s, 1 << bb->shift);
+		target = round_up(target, 1 << bb->shift);
 		sectors = target - s;
 	}
 
-- 
2.43.0

Re: [PATCH v6 1/2] badblocks: fix in-place round_up/round_down usage bug

[Coly Li]

On Thu, Jul 09, 2026 at 06:49:03PM +0800, Ramesh Adhikari wrote:
> rounddown() and roundup() do not modify their first argument in
> place; they return the rounded value. _badblocks_set(),
> _badblocks_clear() and badblocks_check() were calling them as
> bare statements and discarding the result, so 's' (and 'next'/
> 'target') were never actually rounded. Depending on the caller's
> alignment this can leave 'sectors' unchanged or, in the reported
> case, produce a range whose end never advances, causing
> _badblocks_check()/badblocks_check() to loop with a non-advancing
> cursor and stall the CPU (RCU stall) when called through the
> nvdimm ioctl path via nvdimm_clear_badblocks_region().
> 
> rounddown()/roundup() also do division/modulo on the sector_t
> (u64) operand, which requires libgcc helpers (__aeabi_uldivmod,
> __umoddi3) that are not linked into the kernel on 32-bit builds,
> breaking the build on arm/i386 (reported by kernel test robot).
> 
> Switch to round_down()/round_up() (include/linux/math.h), which
> are mask-based, assign their result back to the variable being
> rounded, and require no 64-bit division, fixing both the
> non-rounding bug and the 32-bit build breakage.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202604301231.IpPh4AiH-lkp@intel.com/
> Fixes: aa511ff8218b ("badblocks: switch to the improved badblock handling code")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ramesh Adhikari <adhikari.resume@gmail.com>

It looks good to me.

Reviewed-by: Coly Li <colyli@fygo.io>

Thanks.

Coly Li

> ---
>  block/badblocks.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/block/badblocks.c b/block/badblocks.c
> index ece64e76fe8..1f786b193fb 100644
> --- a/block/badblocks.c
> +++ b/block/badblocks.c
> @@ -857,8 +857,8 @@ static bool _badblocks_set(struct badblocks *bb, sector_t s, sector_t sectors,
>  		/* round the start down, and the end up */
>  		sector_t next = s + sectors;
>  
> -		rounddown(s, 1 << bb->shift);
> -		roundup(next, 1 << bb->shift);
> +		s = round_down(s, 1 << bb->shift);
> +		next = round_up(next, 1 << bb->shift);
>  		sectors = next - s;
>  	}
>  
> @@ -1071,8 +1071,8 @@ static bool _badblocks_clear(struct badblocks *bb, sector_t s, sector_t sectors)
>  		 * isn't than to think a block is not bad when it is.
>  		 */
>  		target = s + sectors;
> -		roundup(s, 1 << bb->shift);
> -		rounddown(target, 1 << bb->shift);
> +		s = round_up(s, 1 << bb->shift);
> +		target = round_down(target, 1 << bb->shift);
>  		sectors = target - s;
>  	}
>  
> @@ -1307,8 +1307,8 @@ int badblocks_check(struct badblocks *bb, sector_t s, sector_t sectors,
>  		/* round the start down, and the end up */
>  		sector_t target = s + sectors;
>  
> -		rounddown(s, 1 << bb->shift);
> -		roundup(target, 1 << bb->shift);
> +		s = round_down(s, 1 << bb->shift);
> +		target = round_up(target, 1 << bb->shift);
>  		sectors = target - s;
>  	}
>  
> -- 
> 2.43.0

[PATCH v6 2/2] badblocks: validate sector range and shift before rounding

[Ramesh Adhikari]

_badblocks_set(), _badblocks_clear() and badblocks_check() round
the caller-supplied [s, s+sectors) range to the current bb->shift
block size before touching the bad block table. That rounding
was not defensive against a few cases:

- s + sectors can overflow sector_t (u64), wrapping the range
  end before it is ever compared against s.

- bb->shift is a plain 'int' field, populated in one case
  (drivers/md/md.c, from the on-disk superblock's bblog_shift)
  straight from an unvalidated byte with no upper bound. Shifting
  by an amount >= the width of the shifted type is undefined
  behaviour in C; "1 << bb->shift" was shifting an int literal,
  so this was already undefined for bb->shift >= 32, let alone
  the full 0-255 range bblog_shift allows.

- round_up()/round_down() rounding a value near ULLONG_MAX can
  itself wrap back to a small value, so even with a valid shift
  the rounded end of the range could end up smaller than the
  rounded start, silently turning a small range into a huge one
  (in _badblocks_clear()/badblocks_check(), which round the end
  up) or losing the range entirely.

Add an explicit s+sectors overflow check, reject any bb->shift
that is too large to shift a sector_t by, cast the shift operand
to sector_t so the shift itself is never performed on a 32-bit
int, and detect post-rounding wrap by comparing the rounded
result back against the pre-rounding value.

Suggested-by: Coly Li <colyli@fygo.io>
Fixes: aa511ff8218b ("badblocks: switch to the improved badblock handling code")
Cc: stable@vger.kernel.org
Signed-off-by: Ramesh Adhikari <adhikari.resume@gmail.com>
---
 block/badblocks.c | 52 ++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 45 insertions(+), 7 deletions(-)

diff --git a/block/badblocks.c b/block/badblocks.c
index 1f786b193fb..00a59729600 100644
--- a/block/badblocks.c
+++ b/block/badblocks.c
@@ -853,12 +853,23 @@ static bool _badblocks_set(struct badblocks *bb, sector_t s, sector_t sectors,
 		/* Invalid sectors number */
 		return false;
 
+	if (s > ULLONG_MAX - sectors)
+		/* Range wraps past the end of sector_t */
+		return false;
+
 	if (bb->shift) {
 		/* round the start down, and the end up */
 		sector_t next = s + sectors;
 
-		s = round_down(s, 1 << bb->shift);
-		next = round_up(next, 1 << bb->shift);
+		if (bb->shift >= BITS_PER_LONG_LONG)
+			/* Corrupt/unsanitised shift value */
+			return false;
+
+		s = round_down(s, (sector_t)1 << bb->shift);
+		next = round_up(next, (sector_t)1 << bb->shift);
+		if (next <= s)
+			/* Rounding wrapped past the end of sector_t */
+			return false;
 		sectors = next - s;
 	}
 
@@ -1061,7 +1072,12 @@ static bool _badblocks_clear(struct badblocks *bb, sector_t s, sector_t sectors)
 		/* Invalid sectors number */
 		return false;
 
+	if (s > ULLONG_MAX - sectors)
+		/* Range wraps past the end of sector_t */
+		return false;
+
 	if (bb->shift) {
+		sector_t orig_s = s;
 		sector_t target;
 
 		/* When clearing we round the start up and the end down.
@@ -1070,10 +1086,21 @@ static bool _badblocks_clear(struct badblocks *bb, sector_t s, sector_t sectors)
 		 * However it is better the think a block is bad when it
 		 * isn't than to think a block is not bad when it is.
 		 */
+		if (bb->shift >= BITS_PER_LONG_LONG)
+			/* Corrupt/unsanitised shift value */
+			return false;
+
 		target = s + sectors;
-		s = round_up(s, 1 << bb->shift);
-		target = round_down(target, 1 << bb->shift);
-		sectors = target - s;
+		s = round_up(s, (sector_t)1 << bb->shift);
+		target = round_down(target, (sector_t)1 << bb->shift);
+		if (s < orig_s || target < s)
+			/* Rounding wrapped, or range collapsed */
+			sectors = 0;
+		else
+			sectors = target - s;
+
+		if (sectors == 0)
+			return false;
 	}
 
 	write_seqlock_irq(&bb->lock);
@@ -1303,12 +1330,23 @@ int badblocks_check(struct badblocks *bb, sector_t s, sector_t sectors,
 
 	WARN_ON(bb->shift < 0 || sectors == 0);
 
+	if (s > ULLONG_MAX - sectors)
+		/* Range wraps past the end of sector_t */
+		return -EINVAL;
+
 	if (bb->shift > 0) {
 		/* round the start down, and the end up */
 		sector_t target = s + sectors;
 
-		s = round_down(s, 1 << bb->shift);
-		target = round_up(target, 1 << bb->shift);
+		if (bb->shift >= BITS_PER_LONG_LONG)
+			/* Corrupt/unsanitised shift value */
+			return -EINVAL;
+
+		s = round_down(s, (sector_t)1 << bb->shift);
+		target = round_up(target, (sector_t)1 << bb->shift);
+		if (target <= s)
+			/* Rounding wrapped past the end of sector_t */
+			return 0;
 		sectors = target - s;
 	}
 
-- 
2.43.0