[PATCH] null_blk: cancel bw_timer on add-device error unwind

[PATCH] null_blk: cancel bw_timer on add-device error unwind

[Cen Zhang]

null_blk starts the bandwidth hrtimer before the later add_disk/device_add
failure points. If setup fails after the timer is queued, the shared error
unwind frees struct nullb without draining bw_timer, so the callback can
run on freed owner state.

The buggy scenario involves two paths, with each column showing the order
within that path:

null_add_dev() error unwind:        nullb_bwtimer_fn() callback path:
1. Start bw_timer for a throttled   1. The hrtimer expires after the free.
   device.                          2. nullb_bwtimer_fn() recovers the
2. Hit a later add_disk/device_add     embedded owner.
   failure.                         3. The callback reads nullb->dev and
3. Free struct nullb.                  nullb->q.
4. Release the remaining queue and   4. The stale owner storage is used
   disk resources.                     after free.

Cancel bw_timer in the shared error unwind before put_disk() and the
remaining frees. The normal delete path already uses the same
hrtimer_cancel() drain.

Validation reproduced this kernel report:
BUG: KASAN: slab-use-after-free in nullb_bwtimer_fn+0x13f/0x170 [null_blk]

Call Trace:
<IRQ>
 dump_stack_lvl+0x66/0xa0
 print_report+0xce/0x630
 ? nullb_bwtimer_fn+0x13f/0x170 [null_blk]
 ? srso_alias_return_thunk+0x5/0xfbef5
 ? __virt_addr_valid+0x20d/0x410
 ? nullb_bwtimer_fn+0x13f/0x170 [null_blk]
 kasan_report+0xe0/0x110
 ? nullb_bwtimer_fn+0x13f/0x170 [null_blk]
 ? __pfx_nullb_bwtimer_fn+0x10/0x10 [null_blk]
 nullb_bwtimer_fn+0x13f/0x170 [null_blk]
 __hrtimer_run_queues+0x172/0x810
 hrtimer_interrupt+0x377/0x7f0
 __sysvec_apic_timer_interrupt+0xc3/0x390
 sysvec_apic_timer_interrupt+0x67/0x80
</IRQ>
 <TASK>
 asm_sysvec_apic_timer_interrupt+0x1a/0x20

Allocated by task 529:
 kasan_save_stack+0x33/0x60
 kasan_save_track+0x14/0x30
 __kasan_kmalloc+0xaa/0xb0
 null_add_dev+0x4f9/0x1d10 [null_blk]
 nullb_device_power_store+0x25f/0x320 [null_blk]
 configfs_write_iter+0x2be/0x4a0
 vfs_write+0x604/0x11f0
 ksys_write+0xf9/0x1d0
 do_syscall_64+0x115/0x6a0
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

Freed by task 529:
 kasan_save_stack+0x33/0x60
 kasan_save_track+0x14/0x30
 kasan_save_free_info+0x3b/0x60
 __kasan_slab_free+0x5f/0x80
 kfree+0x307/0x580
 null_add_dev+0x1272/0x1d10 [null_blk]
 nullb_device_power_store+0x25f/0x320 [null_blk]
 configfs_write_iter+0x2be/0x4a0
 vfs_write+0x604/0x11f0
 ksys_write+0xf9/0x1d0
 do_syscall_64+0x115/0x6a0
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

Fixes: eff2c4f10873 ("nullb: bandwidth control")
Assisted-by: Codex:gpt-5.5
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
 drivers/block/null_blk/main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index f8c0fd57e041..8f1ad76710a0 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -2062,6 +2062,8 @@ static int null_add_dev(struct nullb_device *dev)
 out_ida_free:
 	ida_free(&nullb_indexes, nullb->index);
 out_cleanup_disk:
+	if (test_bit(NULLB_DEV_FL_THROTTLED, &dev->flags))
+		hrtimer_cancel(&nullb->bw_timer);
 	put_disk(nullb->disk);
 out_cleanup_zone:
 	null_free_zoned_dev(dev);
-- 
2.43.0

Re: [PATCH] null_blk: cancel bw_timer on add-device error unwind

[Nilay Shroff]

On 6/24/26 12:48 PM, Cen Zhang wrote:
> null_blk starts the bandwidth hrtimer before the later add_disk/device_add
> failure points. If setup fails after the timer is queued, the shared error
> unwind frees struct nullb without draining bw_timer, so the callback can
> run on freed owner state.
> 
> The buggy scenario involves two paths, with each column showing the order
> within that path:
> 
> null_add_dev() error unwind:        nullb_bwtimer_fn() callback path:
> 1. Start bw_timer for a throttled   1. The hrtimer expires after the free.
>     device.                          2. nullb_bwtimer_fn() recovers the
> 2. Hit a later add_disk/device_add     embedded owner.
>     failure.                         3. The callback reads nullb->dev and
> 3. Free struct nullb.                  nullb->q.
> 4. Release the remaining queue and   4. The stale owner storage is used
>     disk resources.                     after free.
> 
> Cancel bw_timer in the shared error unwind before put_disk() and the
> remaining frees. The normal delete path already uses the same
> hrtimer_cancel() drain.

Looks good to me.
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>