Re: [PATCH v11 08/13] nvme-pci: add SGL support for the dmabuf path

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] nvme-pci: uninitialized rsvd bytes in SGL descriptors
--

commit 4bc1a46f0097646789808ced4d50802904a06015
Author: Anuj Gupta <anuj20.g@samsung.com>

nvme-pci: add SGL support for the dmabuf path

Add SGL support in addition to PRP for dmabuf-backed requests,
building the descriptor list from the mapping's sg_table.

> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index a0506c93fa16e..79b3c1092e4d1 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -1223,12 +1223,18 @@ static blk_status_t nvme_pci_setup_data_prp(struct request *req,
>  	return BLK_STS_IOERR;
>  }
>  
> +static void nvme_pci_sgl_set_data(struct nvme_sgl_desc *sge,
> +		dma_addr_t addr, u32 len)
> +{
> +	sge->addr = cpu_to_le64(addr);
> +	sge->length = cpu_to_le32(len);
> +	sge->type = NVME_SGL_FMT_DATA_DESC << 4;
> +}

[Severity: Medium]
Could this leave the 3-byte rsvd array in the descriptor uninitialized?

The NVMe specification requires the rsvd field in SGL descriptors to be
cleared to 0. Since these descriptors can be allocated via dma_pool_alloc()
with GFP_ATOMIC (which doesn't zero the memory), or reused inline from the
iod->cmd struct, does omitting the initialization here leak prior payload
sizes or physical addresses to the device?

[ ... ]

> @@ -1239,6 +1245,142 @@ static void nvme_pci_sgl_set_seg(struct nvme_sgl_desc *sge,
>  	sge->type = NVME_SGL_FMT_LAST_SEG_DESC << 4;
>  }

[Severity: Medium]
Similarly, does nvme_pci_sgl_set_seg() also need to clear the rsvd field
for SGL segment descriptors to prevent uninitialized data from reaching
the controller?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1791975048.git.asml.silence@gmail.com?part=8