Re: [PATCH v5 3/3] md/raid10: free r10bio before ending master_bio in raid_end_bio_io()

["Chen Cheng" <chencheng@fnnas.com>]

在 2026/6/22 20:29, sashiko-bot@kernel.org 写道:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Incomplete fix: identical Use-After-Free and mempool corruption in `raid_end_discard_bio`.
> - [High] Memory leak of `r10_bio` on `REQ_NOWAIT` write requests during cluster resync or reshape.
> --
> 
> commit 088539a2820315440079f31df4aa792c8a6bc547
> Author: Chen Cheng <chencheng@fnnas.com>
> 
> md/raid10: free r10bio before ending master_bio in raid_end_bio_io()
> 
> This commit reorders the teardown sequence in raid_end_bio_io() to free
> r10_bio before calling bio_endio(). This prevents a race condition where
> a concurrent array reshape can alter the geometry and destroy the mempool
> after the master bio completes but before the r10_bio is freed.
> 
>> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
>> index d740744a97461..a4642c903b202 100644
>> --- a/drivers/md/raid10.c
>> +++ b/drivers/md/raid10.c
>> @@ -332,12 +332,12 @@ static void raid_end_bio_io(struct r10bio *r10_bio)
>>   {
>>   	struct bio *bio = r10_bio->master_bio;
>>   	struct r10conf *conf = r10_bio->mddev->private;
>> +	bool returned = test_and_set_bit(R10BIO_Returned, &r10_bio->state);
>> +	blk_status_t status = test_bit(R10BIO_Uptodate, &r10_bio->state)
>> +				? BLK_STS_OK : BLK_STS_IOERR;
>>   
>> -	if (!test_and_set_bit(R10BIO_Returned, &r10_bio->state)) {
>> -		if (!test_bit(R10BIO_Uptodate, &r10_bio->state))
>> -			bio->bi_status = BLK_STS_IOERR;
>> -		bio_endio(bio);
>> -	}
>> +	put_all_bios(conf, r10_bio);
>> +	mempool_free(r10_bio, conf->r10bio_pool);
>>   
>>   	/*
>>   	 * Wake up any possible resync thread that waits for the device
>> @@ -345,7 +345,10 @@ static void raid_end_bio_io(struct r10bio *r10_bio)
>>   	 */
>>   	allow_barrier(conf);
>>   
>> -	free_r10bio(r10_bio);
>> +	if (!returned) {
>> +		bio->bi_status = status;
>> +		bio_endio(bio);
>> +	}
>>   }
>>   
> 
> [Severity: High]
> This is a pre-existing issue, but the exact same race condition appears to
> exist in the discard completion path.
> 
> In drivers/md/raid10.c:raid_end_discard_bio():
> 
> 	} else {
> 		md_write_end(r10bio->mddev);
> 		bio_endio(r10bio->master_bio);
> 		free_r10bio(r10bio);
> 		break;
> 	}
> 
> Since bio_endio(r10bio->master_bio) drops active_io to zero, couldn't this
> also wake up mddev_suspend() and allow a concurrent reshape to modify
> conf->geo and destroy conf->r10bio_pool before free_r10bio() is called?
> 

yes . raid_end_discard_bio have same issue .

> 
> [Severity: High]
> This is a pre-existing issue, but does raid10_write_request() leak the r10_bio
> on REQ_NOWAIT requests?
> 
> In drivers/md/raid10.c:__make_request():
> 
> 	r10_bio = mempool_alloc(conf->r10bio_pool, GFP_NOIO);
> ...
> 	if (bio_data_dir(bio) == READ)
> 		raid10_read_request(mddev, bio, r10_bio);
> 	else
> 		raid10_write_request(mddev, bio, r10_bio);
> 
> Then in raid10_write_request() during clustered resync:
> 
> 	if ((mddev_is_clustered(mddev) &&
> 	     mddev->cluster_ops->area_resyncing(mddev, WRITE,
> 						bio->bi_iter.bi_sector,
> 						bio_end_sector(bio)))) {
> 		DEFINE_WAIT(w);
> 		/* Bail out if REQ_NOWAIT is set for the bio */
> 		if (bio->bi_opf & REQ_NOWAIT) {
> 			bio_wouldblock_error(bio);
> 			return;
> 		}
> 
> And later during reshape:
> 
> 		if (bio->bi_opf & REQ_NOWAIT) {
> 			allow_barrier(conf);
> 			bio_wouldblock_error(bio);
> 			return;
> 		}
> 
> Since these paths return immediately, they leave the r10_bio allocated. Could
> repeated REQ_NOWAIT rejections eventually exhaust conf->r10bio_pool, causing
> subsequent allocations in __make_request() to hang indefinitely?
> 

seems fixed recently .