Re: [PATCH v5 02/29] md/md-llbitmap: use GFP_NOIO for cache allocations

Thank you for your contribution! Sashiko AI review found 3 potential issue(=
s) to consider:

Pre-existing issues:
- [High] `llbitmap_resize()` recalculates capacity limits but fails to real=
locate the `pctl` array, causing out-of-bounds pointer dereferences during =
subsequent operations.
- [High] `blkdev_issue_zeroout()` is called with `GFP_KERNEL` while `mddev_=
lock()` is held, risking a direct reclaim deadlock.
- [High] The background timer and workqueue can re-arm each other during te=
ardown, leading to a Use-After-Free of the `llbitmap` struct.
--

--- Patch [2]: [PATCH v5 02/29] md/md-llbitmap: use GFP_NOIO for cache allo=
cations ---
commit 0ace115175e6f1ed40865ea8b812e7ca53aadaed
Author: Yu Kuai <[email protected]>

md/md-llbitmap: use GFP_NOIO for cache allocations

This commit changes the memory allocation flags from GFP_KERNEL to GFP_NOIO=
 for
the llbitmap object and its related structures. This change prevents direct
memory reclaim from triggering block I/O while MD resize state locks are he=
ld,
avoiding potential deadlocks.

> llbitmap allocates its in-memory page cache and page-control structures f=
rom
> paths that can already be holding MD reconfiguration or bitmap state lock=
s.
> For example, component_size_store() takes mddev_lock(), update_size() cal=
ls
> the personality resize method, and llbitmap_resize() can grow the page ca=
che
> through llbitmap_prepare_resize().

[Severity: High]
This is a pre-existing issue, but the commit message claims that
llbitmap_resize() grows the page cache through llbitmap_prepare_resize().

Looking at llbitmap_resize():

llbitmap_resize(struct mddev *mddev, sector_t blocks, int chunksize)
{
	...
	llbitmap->chunks =3D chunks;

	return 0;
}

The function updates the capacity limits but does not update llbitmap->nr_p=
ages
or reallocate the llbitmap->pctl array. The referenced llbitmap_prepare_res=
ize()
function does not seem to exist.

Because the pctl array is not reallocated, won't subsequent I/O operations =
call
llbitmap_write() with an index that exceeds the original nr_pages length?

llbitmap_write() calculates the index:

	idx =3D pos >> PAGE_SHIFT;
	bit =3D offset_in_page(pos);

	llbitmap->pctl[idx]->state[bit] =3D state;

Could this lead to an out-of-bounds pointer dereference when accessing the =
new
chunks limit?

> Use GFP_NOIO for the llbitmap object, cached bitmap pages, page controls,
> page-control arrays, and percpu_ref initialization. Leave the explicit
> metadata zeroout path unchanged because it is intentional bitmap I/O rath=
er
> than reclaim-driven allocation.

[Severity: High]
This isn't a bug introduced by this patch, but the commit message explicitly
notes that the metadata zeroout path was left unchanged.

Looking at llbitmap_zero_all_disks(), which is called during array
initialization or reconfiguration with mddev_lock() held:

	ret =3D blkdev_issue_zeroout(rdev->bdev,
				   rdev->data_offset,
				   dev_sectors,
				   GFP_KERNEL, 0);

While the I/O itself is intentional, blkdev_issue_zeroout() uses the provid=
ed
GFP flags internally to allocate the bio and payload pages. If memory is ti=
ght,
won't GFP_KERNEL enter direct reclaim, potentially submitting a bio back to=
 the
same mddev and causing a deadlock?

Should this also be updated to use GFP_NOIO?

> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index 71e9a21b98b2e..3cd8373bc9b22 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c

[ ... ]

[Severity: High]
This is a pre-existing issue, but there appears to be a resource lifecycle
regression during teardown.

Looking at llbitmap_destroy():

	timer_delete_sync(&llbitmap->pending_timer);
	flush_workqueue(md_llbitmap_io_wq);
	...
	kfree(llbitmap);

If the timer had already fired and queued the daemon_work, timer_delete_syn=
c()
returns immediately. flush_workqueue() then blocks waiting for the work ite=
m to
execute.

Inside md_llbitmap_daemon_fn():

	if (restart)
		mod_timer(&llbitmap->pending_timer,
			  jiffies + llbitmap->mddev->bitmap_info.daemon_sleep * HZ);

If unexpired dirty pages are found, it sets restart =3D true and unconditio=
nally
calls mod_timer(), re-arming the timer while the subsystem is undergoing
destruction.

After flush_workqueue() finishes, llbitmap_destroy() frees the llbitmap obj=
ect.

When the newly re-armed timer fires, won't it access the freed object?

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260802195038.1642=
[email protected]?part=3D2