Re: [PATCH 4/5] nvme: decode the FDP Get Features result as a dword

[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:
- [Medium] nvme: uninitialized stack memory use in
  nvme_query_fdp_granularity() [1]
--

commit 37e4c911db178216572cd8ee6fd19cfc2f58c7f0
Author: Shao Mingyin <shao.mingyin@zte.com.cn>

nvme: decode the FDP Get Features result as a dword

This commit changes nvme_query_fdp_info() to read the NVME_FEAT_FDP Get
Features result as a u32 and decode the flags and FDP configuration index
manually so that FDP probing works on big-endian hosts.

> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index a7a4d09644c13..45feafa003257 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -2339,8 +2339,8 @@ static int nvme_query_fdp_info(struct nvme_ns *ns, struct nvme_ns_info *info)
>  	struct nvme_ns_head *head = ns->head;
>  	struct nvme_ctrl *ctrl = ns->ctrl;
>  	struct nvme_fdp_ruh_status *ruhs;
> -	struct nvme_fdp_config fdp;
>  	struct nvme_command c = {};
> +	u32 fdp;
>  	size_t size;
>  	int i, ret;
>  
> @@ -2351,10 +2351,10 @@ static int nvme_query_fdp_info(struct nvme_ns *ns, struct nvme_ns_info *info)
>  		return ret;
>  	}
>  
> -	if (!(fdp.flags & FDPCFG_FDPE))
> +	if (!(fdp & FDPCFG_FDPE))
>  		return 0;
>  
> -	ret = nvme_query_fdp_granularity(ctrl, info, fdp.fdpcidx);
> +	ret = nvme_query_fdp_granularity(ctrl, info, (fdp >> 8) & 0xff);

[Severity: Medium]
This problem wasn't introduced by this patch, but can
nvme_query_fdp_granularity() read uninitialized stack memory from hdr.sze
[1]?

drivers/nvme/host/core.c:nvme_query_fdp_granularity() {
	struct nvme_fdp_config_log hdr, *h;
	...
	ret = nvme_get_log_lsi(ctrl, 0, NVME_LOG_FDP_CONFIGS, 0,
			       NVME_CSI_NVM, &hdr, size, 0, info->endgid);
	if (ret) {
		...
		return ret;
	}

	size = le32_to_cpu(hdr.sze);
	...
	h = kvzalloc(size, GFP_KERNEL);
}

hdr is allocated on the stack without zero-initialization.  If the
controller completes the log page command with a short data transfer,
hdr.sze remains uninitialized and is passed as the allocation size to
kvzalloc().

[1] https://sashiko.dev/#/bug/linux-1cda5d44-a6cc-40a9-bd2b-cbed1b364093

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/202611150127512368BGQ07ZNIRAIwClkam-8u@zte.com.cn?part=4