Re: [PATCH next] spi: Fix potential uninitialized variable in probe()

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1773 bytes --]

On Fri, Nov 21, 2025 at 02:18:49PM +0000, Mark Brown wrote:
> On Fri, Nov 21, 2025 at 04:35:01PM +0300, Dan Carpenter wrote:
> > If the device tree is messed up, then potentially the "protocol" string
> > could potentially be uninitialized.  Add a check to prevent that.
> > 
> > Fixes: 059f545832be ("spi: add support for microchip "soft" spi controller")
> > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> > ---
> >  drivers/spi/spi-microchip-core-spi.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/spi/spi-microchip-core-spi.c b/drivers/spi/spi-microchip-core-spi.c
> > index b8738190cdcb..e65036cc62f3 100644
> > --- a/drivers/spi/spi-microchip-core-spi.c
> > +++ b/drivers/spi/spi-microchip-core-spi.c
> > @@ -320,6 +320,8 @@ static int mchp_corespi_probe(struct platform_device *pdev)
> >  	 */
> >  	ret = of_property_read_string(pdev->dev.of_node, "microchip,protocol-configuration",
> >  				      &protocol);
> > +	if (ret)
> > +		return ret;
> >  	if (strcmp(protocol, "motorola") != 0)
> >  		return dev_err_probe(&pdev->dev, -EINVAL,
> >  				     "CoreSPI: protocol '%s' not supported by this driver\n",
> 
> This should probably also complain about not being able to get the
> property, otherwise nobody is going to be able to figure out what's
> wrong if we actually hit the error case.

The one thing to be careful of is that the property has a default, so
EINVAL needs to be treated differently, so the decision tree is
something like:
if (ret == _EINVAL)
	<do nothing>
else if (ret)
	abort complaining about malformed
else if (!motorola)
	abort complaining about unsupported mode
else
	<do nothing>

obviously that can just become two clauses, but you get the idea.




[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]