From 940d448198c56c305a4eb5bfb410330ed30af430 Mon Sep 17 00:00:00 2001 From: Ramesh Adhikari Date: Sat, 4 Jul 2026 22:43:21 +0530 Subject: [PATCH] badblocks: fix infinite loop due to incorrect rounding and overflow 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 --- block/badblocks.c | 40 ++++++++++++++++++++++++++++++++-------- 1 file changed, 32 insertions(+), 8 deletions(-) diff --git a/block/badblocks.c b/block/badblocks.c index ece64e76fe8ff..e848491253235 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: