Re: [PATCH 01/23] nvmet: add context annotation for nvmet_ns::bdev

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Critical] nvmet: race in nvmet_bdev_zone_append_bio_done() on
  inline_bio cleanup [1]
--

commit bd98b59eeb25b1fa920c66a35d00856fefddae58
Author: Nilay Shroff <nilay@linux.ibm.com>

nvmet: add context annotation for nvmet_ns::bdev

This commit annotates nvmet_ns::bdev with __guarded_by(&subsys->lock), adds
__must_hold() annotations to namespace management functions, and introduces
the nvmet_ns_bdev() helper for lockless I/O and request-processing paths.

> diff --git a/drivers/nvme/target/zns.c b/drivers/nvme/target/zns.c
> index cea05b4bc7409..ce53094536688 100644
> --- a/drivers/nvme/target/zns.c
> +++ b/drivers/nvme/target/zns.c

[ ... ]

> @@ -591,10 +593,10 @@ void nvmet_bdev_execute_zone_append(struct nvmet_req *req)
>  
>  	if (nvmet_use_inline_bvec(req)) {
>  		bio = &req->z.inline_bio;
> -		bio_init(bio, req->ns->bdev, req->inline_bvec,
> +		bio_init(bio, bdev, req->inline_bvec,
>  			 ARRAY_SIZE(req->inline_bvec), opf);
>  	} else {
> -		bio = bio_alloc(req->ns->bdev, req->sg_cnt, opf, GFP_KERNEL);
> +		bio = bio_alloc(bdev, req->sg_cnt, opf, GFP_KERNEL);
>  	}
>  
>  	bio->bi_end_io = nvmet_bdev_zone_append_bio_done;

[Severity: Critical]
This problem wasn't introduced by this patch, but can
nvmet_bdev_zone_append_bio_done() cause a use-after-free on req or corrupt
req->z.inline_bio [1]?

drivers/nvme/target/zns.c:nvmet_bdev_zone_append_bio_done() {
	...
	nvmet_req_complete(req, blk_to_nvme_status(req, bio->bi_status));
	nvmet_req_bio_put(req, bio);
}

nvmet_req_complete() invokes req->ops->queue_response(), allowing the
transport to immediately free req or reuse it for a new command before
nvmet_req_bio_put() runs:

CPU0                                  CPU1
nvmet_bdev_zone_append_bio_done()
  nvmet_req_complete(req, ...)
    req->ops->queue_response(req)
                                      bio_init(&req->z.inline_bio, ...)
                                      submit_bio(&req->z.inline_bio)
  nvmet_req_bio_put(req, bio)
    bio_uninit(bio)
      bio->bi_blkg = NULL
                                        blk_cgroup_bio_start(bio)
                                          <-- NULL pointer dereference

Saving bio->bi_status and calling nvmet_req_bio_put() before
nvmet_req_complete(), as nvmet_bio_done() does, would avoid touching req and
inline_bio after completion.

[1] https://sashiko.dev/#/bug/linux-26aa255a-7ec1-4018-9aa1-cc5794fc39c0

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109033102.982191-25-nilay@linux.ibm.com?part=1