[PATCH 1/1] nvme: fix implicit bool to flags conversion

[PATCH 1/1] nvme: fix implicit bool to flags conversion

[Pavel Begunkov]

nvme_map_user_request() takes flags as the last argument, but
nvme_uring_cmd_io() shoves a bool "vec" into it. It behaves as
expected because bool is converted to 0/1 and NVME_IOCTL_VEC is
defined as 1, but it's better to pass flags explicitly.

Fixes: 7b7fdb8e2dbc1 ("nvme: replace the "bool vec" arguments with flags in the ioctl path")
Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 drivers/nvme/host/ioctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
index ca86d3bf7ea4..f29107d95ff2 100644
--- a/drivers/nvme/host/ioctl.c
+++ b/drivers/nvme/host/ioctl.c
@@ -521,7 +521,7 @@ static int nvme_uring_cmd_io(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
 	if (d.data_len) {
 		ret = nvme_map_user_request(req, d.addr, d.data_len,
 			nvme_to_user_ptr(d.metadata), d.metadata_len,
-			map_iter, vec);
+			map_iter, vec ? NVME_IOCTL_VEC : 0);
 		if (ret)
 			goto out_free_req;
 	}
-- 
2.49.0

Re: [PATCH 1/1] nvme: fix implicit bool to flags conversion

[Jens Axboe]

On 5/20/25 9:14 AM, Pavel Begunkov wrote:
> nvme_map_user_request() takes flags as the last argument, but
> nvme_uring_cmd_io() shoves a bool "vec" into it. It behaves as
> expected because bool is converted to 0/1 and NVME_IOCTL_VEC is
> defined as 1, but it's better to pass flags explicitly.

LGTM:

Reviewed-by: Jens Axboe <axboe@kernel.dk>

-- 
Jens Axboe

Re: [PATCH 1/1] nvme: fix implicit bool to flags conversion

[Anuj gupta]

Looks good to me.
Reviewed-by: Anuj Gupta <anuj20.g@samsung.com>

Re: [PATCH 1/1] nvme: fix implicit bool to flags conversion

[Caleb Sander Mateos]

On Tue, May 20, 2025 at 8:28 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
>
> nvme_map_user_request() takes flags as the last argument, but
> nvme_uring_cmd_io() shoves a bool "vec" into it. It behaves as
> expected because bool is converted to 0/1 and NVME_IOCTL_VEC is
> defined as 1, but it's better to pass flags explicitly.
>
> Fixes: 7b7fdb8e2dbc1 ("nvme: replace the "bool vec" arguments with flags in the ioctl path")
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>

Keith had an earlier patch to fix this, but looks like the series
never got merged:
https://lore.kernel.org/linux-nvme/20250224182128.2042061-4-kbusch@meta.com/
This approach also looks fine to me, but not sure if it will
complicate some of the changes Keith wanted to make.

Reviewed-by: Caleb Sander Mateos <csander@purestorage.com>

Re: [PATCH 1/1] nvme: fix implicit bool to flags conversion

[Keith Busch]

On Tue, May 20, 2025 at 08:42:36AM -0700, Caleb Sander Mateos wrote:
> On Tue, May 20, 2025 at 8:28 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
> >
> > nvme_map_user_request() takes flags as the last argument, but
> > nvme_uring_cmd_io() shoves a bool "vec" into it. It behaves as
> > expected because bool is converted to 0/1 and NVME_IOCTL_VEC is
> > defined as 1, but it's better to pass flags explicitly.
> >
> > Fixes: 7b7fdb8e2dbc1 ("nvme: replace the "bool vec" arguments with flags in the ioctl path")
> > Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> 
> Keith had an earlier patch to fix this, but looks like the series
> never got merged:
> https://lore.kernel.org/linux-nvme/20250224182128.2042061-4-kbusch@meta.com/
> This approach also looks fine to me, but not sure if it will
> complicate some of the changes Keith wanted to make.

Thanks, but no need to wait on me for this. I will revisit these
cleanups someday once I can arrange for fewer distractions, which may
not be any time soon.

Patch looks good to me too.

Reviewed-by: Keith Busch <kbusch@kernel.org>

Re: [PATCH 1/1] nvme: fix implicit bool to flags conversion

[Kanchan Joshi]

Reviewed-by: Kanchan Joshi <joshi.k@samsung.com>

Re: [PATCH 1/1] nvme: fix implicit bool to flags conversion

[Chaitanya Kulkarni]

On 5/20/25 08:14, Pavel Begunkov wrote:
> nvme_map_user_request() takes flags as the last argument, but
> nvme_uring_cmd_io() shoves a bool "vec" into it. It behaves as
> expected because bool is converted to 0/1 and NVME_IOCTL_VEC is
> defined as 1, but it's better to pass flags explicitly.
>
> Fixes: 7b7fdb8e2dbc1 ("nvme: replace the "bool vec" arguments with flags in the ioctl path")
> Signed-off-by: Pavel Begunkov<asml.silence@gmail.com>

Looks good.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck

Re: [PATCH 1/1] nvme: fix implicit bool to flags conversion

[Pavel Begunkov]

On 5/20/25 16:46, Keith Busch wrote:
> On Tue, May 20, 2025 at 08:42:36AM -0700, Caleb Sander Mateos wrote:
>> On Tue, May 20, 2025 at 8:28 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
>>>
>>> nvme_map_user_request() takes flags as the last argument, but
>>> nvme_uring_cmd_io() shoves a bool "vec" into it. It behaves as
>>> expected because bool is converted to 0/1 and NVME_IOCTL_VEC is
>>> defined as 1, but it's better to pass flags explicitly.
>>>
>>> Fixes: 7b7fdb8e2dbc1 ("nvme: replace the "bool vec" arguments with flags in the ioctl path")
>>> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
>>
>> Keith had an earlier patch to fix this, but looks like the series
>> never got merged:
>> https://lore.kernel.org/linux-nvme/20250224182128.2042061-4-kbusch@meta.com/
>> This approach also looks fine to me, but not sure if it will
>> complicate some of the changes Keith wanted to make.

Aha, so it was caught before, that's good.


> Thanks, but no need to wait on me for this. I will revisit these
> cleanups someday once I can arrange for fewer distractions, which may
> not be any time soon.
> 
> Patch looks good to me too.
> 
> Reviewed-by: Keith Busch <kbusch@kernel.org>
In which case it'd make sense to take this patch, but no objection
in either case.

-- 
Pavel Begunkov

Re: [PATCH 1/1] nvme: fix implicit bool to flags conversion

[Christoph Hellwig]

Thanks,

applied to nvme-6.16.