Fix the uninitialized symbol 'bio' in the function blk_rq_prep_clone
to resolve the following error:
block/blk-mq.c:3199 blk_rq_prep_clone() error: uninitialized symbol 'bio'.

Signed-off-by: SurajSonawane2415 <surajsonawane0215@gmail.com>
---
V1 - Initialize 'bio' to NULL.
V2 - Move bio_put(bio) into the bio_ctr error handling block, 
ensuring memory cleanup occurs only when the bio_ctr fail.
V3 - Moved the bio declaration into the loop scope, eliminating
the need to set it to NULL at the end of the loop.

 block/blk-mq.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 4b2c8e940..27b22dbfc 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -3156,19 +3156,21 @@ int blk_rq_prep_clone(struct request *rq, struct request *rq_src,
 		      int (*bio_ctr)(struct bio *, struct bio *, void *),
 		      void *data)
 {
-	struct bio *bio, *bio_src;
+	struct bio *bio_src;
 
 	if (!bs)
 		bs = &fs_bio_set;
 
 	__rq_for_each_bio(bio_src, rq_src) {
-		bio = bio_alloc_clone(rq->q->disk->part0, bio_src, gfp_mask,
+		struct bio *bio = bio_alloc_clone(rq->q->disk->part0, bio_src, gfp_mask,
 				      bs);
 		if (!bio)
 			goto free_and_out;
 
-		if (bio_ctr && bio_ctr(bio, bio_src, data))
+		if (bio_ctr && bio_ctr(bio, bio_src, data)) {
+			bio_put(bio);
 			goto free_and_out;
+		}
 
 		if (rq->bio) {
 			rq->biotail->bi_next = bio;
@@ -3176,7 +3178,6 @@ int blk_rq_prep_clone(struct request *rq, struct request *rq_src,
 		} else {
 			rq->bio = rq->biotail = bio;
 		}
-		bio = NULL;
 	}
 
 	/* Copy attributes of the original request to the clone request. */
@@ -3196,8 +3197,6 @@ int blk_rq_prep_clone(struct request *rq, struct request *rq_src,
 	return 0;
 
 free_and_out:
-	if (bio)
-		bio_put(bio);
 	blk_rq_unprep_clone(rq);
 
 	return -ENOMEM;
-- 
2.34.1

On Tue, Oct 08, 2024 at 05:34:13PM +0530, SurajSonawane2415 wrote:
> Fix the uninitialized symbol 'bio' in the function blk_rq_prep_clone
> to resolve the following error:
> block/blk-mq.c:3199 blk_rq_prep_clone() error: uninitialized symbol 'bio'.

...

> @@ -3156,19 +3156,21 @@ int blk_rq_prep_clone(struct request *rq, struct request *rq_src,
>  		      int (*bio_ctr)(struct bio *, struct bio *, void *),
>  		      void *data)
>  {
> -	struct bio *bio, *bio_src;
> +	struct bio *bio_src;
>  
>  	if (!bs)
>  		bs = &fs_bio_set;
>  
>  	__rq_for_each_bio(bio_src, rq_src) {
> -		bio = bio_alloc_clone(rq->q->disk->part0, bio_src, gfp_mask,
> +		struct bio *bio = bio_alloc_clone(rq->q->disk->part0, bio_src, gfp_mask,
>  				      bs);
>  		if (!bio)
>  			goto free_and_out;
>  
> -		if (bio_ctr && bio_ctr(bio, bio_src, data))
> +		if (bio_ctr && bio_ctr(bio, bio_src, data)) {
> +			bio_put(bio);
>  			goto free_and_out;
> +		}
>  
>  		if (rq->bio) {
>  			rq->biotail->bi_next = bio;
> @@ -3176,7 +3178,6 @@ int blk_rq_prep_clone(struct request *rq, struct request *rq_src,
>  		} else {
>  			rq->bio = rq->biotail = bio;
>  		}
> -		bio = NULL;
>  	}
>  
>  	/* Copy attributes of the original request to the clone request. */
> @@ -3196,8 +3197,6 @@ int blk_rq_prep_clone(struct request *rq, struct request *rq_src,
>  	return 0;
>  
>  free_and_out:
> -	if (bio)
> -		bio_put(bio);
>  	blk_rq_unprep_clone(rq);

I think your commit message is missing the real "fix" here. The other
place that goto's this label is if blk_crypto_rq_bio_prep() fails. At
this point, the cloned 'rq' has all the bio's that get cleaned up in
blk_rq_unprep_clone(), so that failure scenario is double put'ing the
last bio.

On Tue, Oct 08, 2024 at 08:52:07AM -0600, Keith Busch wrote:
> I think your commit message is missing the real "fix" here. The other
> place that goto's this label is if blk_crypto_rq_bio_prep() fails. At
> this point, the cloned 'rq' has all the bio's that get cleaned up in
> blk_rq_unprep_clone(), so that failure scenario is double put'ing the
> last bio.

Ah, forget that. The existing code was NULL'ing the bio before
prep_clone, so the scenario I described doesn't happen.

> +		struct bio *bio = bio_alloc_clone(rq->q->disk->part0, bio_src, gfp_mask,
>  				      bs);

Overly long line here, plus now pretty weird positioning of the bs
argument.

Should be something like:

		struct bio *bio = bio_alloc_clone(rq->q->disk->part0, bio_src,
				gfp_mask, bs);