[PATCH] nvme: unmap the data buffer when metadata mapping fails

[PATCH] nvme: unmap the data buffer when metadata mapping fails

[Joel Granados]

Commit d0d1d522316e ("blk-map: provide the bdev to bio if one exists")
dropped the "bio = req->bio" assignment in nvme_map_user_request(), but
left the local bio variable initialized to NULL and still used it in the
out_unmap error path.  The "if (bio)" test is therefore always false, so
a failure of blk_rq_integrity_map_user() no longer unmaps the already
mapped data buffer.  The callers only call blk_mq_free_request(), which
does not unmap user pages, leaking the bio and its pinned user pages.

Use req->bio directly to unmap the data buffer on the error path, and
drop the now unused local variable.

Fixes: d0d1d522316e ("blk-map: provide the bdev to bio if one exists")

Signed-off-by: Joel Granados <joel.granados@kernel.org>
---
Did we forget to unmap?
---
 drivers/nvme/host/ioctl.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
index 9597a87cf05dc32a7eb0373485f575502c32a105..9ae3c0aadfb8f35790c8e57619d1af69ca41af0c 100644
--- a/drivers/nvme/host/ioctl.c
+++ b/drivers/nvme/host/ioctl.c
@@ -122,7 +122,6 @@ static int nvme_map_user_request(struct request *req, u64 ubuffer,
 	bool supports_metadata = bdev && blk_get_integrity(bdev->bd_disk);
 	struct nvme_ctrl *ctrl = nvme_req(req)->ctrl;
 	bool has_metadata = meta_buffer && meta_len;
-	struct bio *bio = NULL;
 	int ret;
 
 	if (!nvme_ctrl_sgl_supported(ctrl))
@@ -154,8 +153,8 @@ static int nvme_map_user_request(struct request *req, u64 ubuffer,
 	return ret;
 
 out_unmap:
-	if (bio)
-		blk_rq_unmap_user(bio);
+	if (req->bio)
+		blk_rq_unmap_user(req->bio);
 	return ret;
 }
 

---
base-commit: adeac771f4901bb66267eaddb9fcc538925f92a4
change-id: 20260612-jag-fixes-6b459ea16d53

Best regards,
-- 
Joel Granados <joel.granados@kernel.org>

Re: [PATCH] nvme: unmap the data buffer when metadata mapping fails

[Maurizio Lombardi]

On Fri Jun 12, 2026 at 11:40 AM CEST, Joel Granados wrote:
> Commit d0d1d522316e ("blk-map: provide the bdev to bio if one exists")
> dropped the "bio = req->bio" assignment in nvme_map_user_request(), but
> left the local bio variable initialized to NULL and still used it in the
> out_unmap error path.  The "if (bio)" test is therefore always false, so
> a failure of blk_rq_integrity_map_user() no longer unmaps the already
> mapped data buffer.  The callers only call blk_mq_free_request(), which
> does not unmap user pages, leaking the bio and its pinned user pages.
>
> Use req->bio directly to unmap the data buffer on the error path, and
> drop the now unused local variable.
>
> Fixes: d0d1d522316e ("blk-map: provide the bdev to bio if one exists")
>
> Signed-off-by: Joel Granados <joel.granados@kernel.org>
> ---
> Did we forget to unmap?
> ---

Isn't this already fixed in mainline tree?

2279cd9c61a330e5 ("nvme: fix bio leak on mapping failure")

Maurizio

>  drivers/nvme/host/ioctl.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
> index 9597a87cf05dc32a7eb0373485f575502c32a105..9ae3c0aadfb8f35790c8e57619d1af69ca41af0c 100644
> --- a/drivers/nvme/host/ioctl.c
> +++ b/drivers/nvme/host/ioctl.c
> @@ -122,7 +122,6 @@ static int nvme_map_user_request(struct request *req, u64 ubuffer,
>  	bool supports_metadata = bdev && blk_get_integrity(bdev->bd_disk);
>  	struct nvme_ctrl *ctrl = nvme_req(req)->ctrl;
>  	bool has_metadata = meta_buffer && meta_len;
> -	struct bio *bio = NULL;
>  	int ret;
>  
>  	if (!nvme_ctrl_sgl_supported(ctrl))
> @@ -154,8 +153,8 @@ static int nvme_map_user_request(struct request *req, u64 ubuffer,
>  	return ret;
>  
>  out_unmap:
> -	if (bio)
> -		blk_rq_unmap_user(bio);
> +	if (req->bio)
> +		blk_rq_unmap_user(req->bio);
>  	return ret;
>  }
>  
>
> ---
> base-commit: adeac771f4901bb66267eaddb9fcc538925f92a4
> change-id: 20260612-jag-fixes-6b459ea16d53
>
> Best regards,

Re: [PATCH] nvme: unmap the data buffer when metadata mapping fails

[Joel Granados]

[-- Attachment #1: Type: text/plain, Size: 1246 bytes --]

On Fri, Jun 12, 2026 at 11:45:26AM +0200, Maurizio Lombardi wrote:
> On Fri Jun 12, 2026 at 11:40 AM CEST, Joel Granados wrote:
> > Commit d0d1d522316e ("blk-map: provide the bdev to bio if one exists")
> > dropped the "bio = req->bio" assignment in nvme_map_user_request(), but
> > left the local bio variable initialized to NULL and still used it in the
> > out_unmap error path.  The "if (bio)" test is therefore always false, so
> > a failure of blk_rq_integrity_map_user() no longer unmaps the already
> > mapped data buffer.  The callers only call blk_mq_free_request(), which
> > does not unmap user pages, leaking the bio and its pinned user pages.
> >
> > Use req->bio directly to unmap the data buffer on the error path, and
> > drop the now unused local variable.
> >
> > Fixes: d0d1d522316e ("blk-map: provide the bdev to bio if one exists")
> >
> > Signed-off-by: Joel Granados <joel.granados@kernel.org>
> > ---
> > Did we forget to unmap?
> > ---
> 
> Isn't this already fixed in mainline tree?
That is indeed the case. I must have been looking at an outdated state.

Sorry for the noise.

Best
> 
> 2279cd9c61a330e5 ("nvme: fix bio leak on mapping failure")
> 
> Maurizio
> 

-- 

Joel Granados

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]