Re: [PATCH] md/raid1: release barrier when REQ_NOWAIT write would block

On Thu, Jun 11, 2026 at 13:41 +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> Pre-existing issues:
> - [Critical] Missing RCU read lock around `conf->mirrors[i].rdev` access in `wait_blocked_rdev()`.
I'm not quite sure whether this is a real bug.

> - [High] Early failure paths in `raid1_write_request()` leak the `mddev->writes_pending` reference.
I'll submit a v2 containing this fix, along with the fix for the
mddev->writes_pending

> - [High] Allocating bios and r1bios can sleep even when the `REQ_NOWAIT` flag is set.
@kuai, Should I handle this issue as well, or should I just ignore it
for now?

> --
>
> commit 3db9da0ec804800aa3afb93361ccd18de39ce5f0
> Author: Abd-Alrhman Masalkhi <[email protected]>
>
> md/raid1: release barrier when REQ_NOWAIT write would block
>
> raid1_write_request() leaks the barrier reference taken by wait_barrier()
> if wait_blocked_rdev() fails for a REQ_NOWAIT write. This patch releases
> the barrier on this failure path to prevent array freeze hangs.
>
> Closes: https://sashiko.dev/#/patchset/[email protected]?part=1
>
>> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
>> index 5b9368bd9e700..f3cc30932ba2b 100644
>> --- a/drivers/md/raid1.c
>> +++ b/drivers/md/raid1.c
>> @@ -1512,6 +1512,7 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
>>  	int max_sectors;
>>  	bool write_behind = false;
>>  	bool is_discard = (bio_op(bio) == REQ_OP_DISCARD);
>> +	sector_t sector = bio->bi_iter.bi_sector;
>>  
>>  	if (mddev_is_clustered(mddev) &&
>>  	    mddev->cluster_ops->area_resyncing(mddev, WRITE,
>> @@ -1540,6 +1541,7 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
>>  
>>  	if (!wait_blocked_rdev(mddev, bio)) {
>>  		bio_wouldblock_error(bio);
>> +		allow_barrier(conf, sector);
>
> [Severity: High]
> This is a pre-existing issue, but does this early return path also leak the
> mddev->writes_pending reference?
>
> In raid1_make_request(), md_write_start() unconditionally takes a per-cpu
> reference on mddev->writes_pending before calling raid1_write_request().
> Normally, this is released via md_write_end() inside close_write().
>
> By directly returning here after bio_wouldblock_error(bio), we never call
> md_write_end(). Does this permanently leak the writes_pending counter and
> prevent the array from transitioning to a clean state?
>
> It looks like this same leak exists on the clustered area_resyncing and
> wait_barrier REQ_NOWAIT failure paths earlier in this function, as well as
> the err_handle path if bio_submit_split_bioset() fails.
>
>>  		return;
>>  	}
>
> [Severity: Critical]
> This is a pre-existing issue, but is it safe to call wait_blocked_rdev()
> here without holding the RCU read lock?
>
> Looking at wait_blocked_rdev():
>
> 	for (i = 0; i < disks; i++) {
> 		struct md_rdev *rdev = conf->mirrors[i].rdev;
>
> 		if (!rdev)
> 			continue;
>
> 		/* don't write here until the bad block is acknowledged */
> 		if (test_bit(WriteErrorSeen, &rdev->flags) &&
>
> It accesses the rdev pointer and its fields without rcu_read_lock().
> Could a concurrent device removal (via md_ioctl -> raid1_remove_conf)
> replace the pointer with NULL and free the rdev structure while we are
> dereferencing it here, resulting in a use-after-free?
>
> [Severity: High]
> This is also a pre-existing issue, but if we proceed past this point for a
> REQ_NOWAIT bio, we call alloc_r1bio() and eventually bio_alloc_clone(),
> both of which use GFP_NOIO.
>
> Since memory pool allocations with GFP_NOIO can block when the mempool is
> exhausted under memory pressure, could this cause REQ_NOWAIT writes to
> sleep, defeating the purpose of the REQ_NOWAIT flag?
>
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

-- 
Best Regards,
Abd-Alrhman