Re: [PATCH v2] cxl/mbox: bound the Get Supported Logs entry count by the payload

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] cxl/mbox: memory leak of gsl buffer on truncated payload error path
--

commit 7ecb0885e04d5e150bd0769634b9a3391c017244
Author: Gaobin Huang <huanggaobin23@semi.ac.cn>
cxl/mbox: bound the Get Supported Logs entry count by the payload

This commit adds bounds checking for the number of entries in the Get
Supported Logs response against the actual payload size. It prevents
out-of-bounds reads during device probe by ensuring the device doesn't
report more entries than it actually returns.

> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
[ ... ]
> @@ -849,18 +851,47 @@ int cxl_enumerate_cmds(struct cxl_memdev_state *mds)
>  	struct cxl_mbox_get_supported_logs *gsl;
>  	struct device *dev = mds->cxlds.dev;
>  	struct cxl_mem_command *cmd;
> +	size_t gsl_len, gsl_hdr_size, max_entries;
>  	int i, rc;
>  
> -	gsl = cxl_get_gsl(mds);
> +	gsl = cxl_get_gsl(mds, &gsl_len);
>  	if (IS_ERR(gsl))
>  		return PTR_ERR(gsl);
>  
> +	/*
> +	 * The device chooses the reported payload length and min_out only
> +	 * requires the entry count field on its own (2 bytes), so a response
> +	 * shorter than the header is reachable.  There is nothing to enumerate
> +	 * in that case: fail rather than derive a bound from an underflowed
> +	 * subtraction.
> +	 */
> +	gsl_hdr_size = struct_offset(gsl, entry);
> +	if (gsl_len < gsl_hdr_size) {
> +		dev_err(dev,
> +		"GSL: response of %zu bytes is too short for the header\n",
> +		gsl_len);
> +		return -EIO;

[Severity: Medium]
Does this code leak the `gsl` buffer allocated by cxl_get_gsl()?

cxl_get_gsl() dynamically allocates the `gsl` buffer using kvmalloc() and
returns it. By directly returning -EIO when the payload is shorter than the
header, we bypass the `goto out;` block which contains the necessary
kvfree(gsl) call. Since this occurs on a device probe error path, it leaks
the allocated memory.

As an additional observation, leaving `.min_out = 2` in cxl_get_gsl() forces
higher layers to manually re-validate the payload length here. Setting
`.min_out = 8` (or using struct_size()) in cxl_get_gsl() would allow the CXL
mailbox core to handle this validation natively without requiring these
manual checks.

> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917104603.2658529-1-huanggaobin23@semi.ac.cn?part=1