| Message ID | 20241008120413.16402-1-surajsonawane0215@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] block: Fix uninitialized symbol 'bio' in blk_rq_prep_clone | expand |
> + 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);
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.
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;
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(-)