Re: [PATCH RFC v3 1/7] mfd: ls2kbmc: Make a copy when parsing mode string

[Lee Jones]

Why is this v3 set still RFC?  What comment are you requesting?

On Fri, 10 Jul 2026, Miao Wang via B4 Relay wrote:

> From: Miao Wang <shankerwangmiao@xxxxxxxxx>
> 
> When parsing the mode string from BMC, the string is manipulated
> in-place with strsep(), preventing from parsing it again. Make a copy of
> the original string and manipulate the copy instead to fix this.
> 
> Fixes: 0d64f6d1ffe9 ("mfd: ls2kbmc: Introduce Loongson-2K BMC core driver")
> Signed-off-by: Miao Wang <shankerwangmiao@xxxxxxxxx>
> ---
>  drivers/mfd/ls2k-bmc-core.c | 30 +++++++++++++++++++++++++-----
>  1 file changed, 25 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mfd/ls2k-bmc-core.c b/drivers/mfd/ls2k-bmc-core.c
> index 408056bfb2fe757a5bde43775a483a48352e706d..27f6e096404d67459038a0607378057ec7ef69ab 100644
> --- a/drivers/mfd/ls2k-bmc-core.c
> +++ b/drivers/mfd/ls2k-bmc-core.c
> @@ -427,34 +427,54 @@ static int ls2k_bmc_init(struct ls2k_bmc_ddata *ddata)
>   */
>  static int ls2k_bmc_parse_mode(struct pci_dev *pdev, struct simplefb_platform_data *pd)
>  {
> -	char *mode;
> +	/* Assume 64 bytes is enough for the resolution string */
> +	char mode_buf[64], mode_buf_orig[64];
> +	char *mode = mode_buf;
> +	const void __iomem *mode_base;
>  	int depth, ret;
>  
>  	/* The last 16M of PCI BAR0 is used to store the resolution string. */
> -	mode = devm_ioremap(&pdev->dev, pci_resource_start(pdev, 0) + SZ_16M, SZ_16M);
> -	if (!mode)
> +	mode_base = ioremap(pci_resource_start(pdev, 0) + SZ_16M,
> +			    sizeof(mode_buf));
> +	if (!mode_base)
>  		return -ENOMEM;
> +	memcpy_fromio(mode_buf, mode_base, sizeof(mode_buf) - 1);
> +	mode_buf[sizeof(mode_buf) - 1] = '\0';
> +	iounmap((void __iomem *)mode_base);
> +	memcpy(mode_buf_orig, mode_buf, sizeof(mode_buf_orig));
>  
>  	/* The resolution field starts with the flag "video=". */
>  	if (!strncmp(mode, "video=", 6))
>  		mode = mode + 6;
> +	else
> +		goto invalid_mode;
>  
> -	ret = kstrtoint(strsep(&mode, "x"), 10, &pd->width);
> +	ret = kstrtouint(strsep(&mode, "x"), 10, &pd->width);
>  	if (ret)
>  		return ret;
> +	if (mode == NULL)
> +		goto invalid_mode;
>  
> -	ret = kstrtoint(strsep(&mode, "-"), 10, &pd->height);
> +	ret = kstrtouint(strsep(&mode, "-"), 10, &pd->height);
>  	if (ret)
>  		return ret;
> +	if (mode == NULL)
> +		goto invalid_mode;
>  
>  	ret = kstrtoint(strsep(&mode, "@"), 10, &depth);
>  	if (ret)
>  		return ret;
> +	if (mode == NULL)
> +		goto invalid_mode;
>  
>  	pd->stride = pd->width * depth / 8;
>  	pd->format = depth == 32 ? "a8r8g8b8" : "r5g6b5";
>  
>  	return 0;
> +
> +invalid_mode:
> +	dev_err(&pdev->dev, "Invalid resolution string: %s\n", mode_buf_orig);
> +	return -EINVAL;
>  }
>  
>  static int ls2k_bmc_probe(struct pci_dev *dev, const struct pci_device_id *id)
> 
> -- 
> 2.49.0
> 
> 

-- 
Lee Jones