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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aSQPto0zcTniM85G@stanley.mountain>
Date: Mon, 24 Nov 2025 10:56:38 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Conor Dooley <conor@...nel.org>
Cc: Mark Brown <broonie@...nel.org>,
	Prajna Rajendra Kumar <prajna.rajendrakumar@...rochip.com>,
	Conor Dooley <conor.dooley@...rochip.com>,
	linux-spi@...r.kernel.org, linux-kernel@...r.kernel.org,
	kernel-janitors@...r.kernel.org
Subject: Re: [PATCH next] spi: Fix potential uninitialized variable in probe()

On Fri, Nov 21, 2025 at 04:20:41PM +0000, Conor Dooley wrote:
> 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@...aro.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.

Sure.  I've sent a v2 which defaults to motorola.

regards,
dan carpenter