linux-kernel - RE: [PATCH 5/8] mmc: sdhci-of-aspeed: Fix null pointer

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] [thread-next>] [day] [month] [year] [list]

Message-ID:
 <SEYPR06MB7072DB0E72EBCE12FE6A78CE957CA@SEYPR06MB7072.apcprd06.prod.outlook.com>
Date: Fri, 20 Jun 2025 08:18:58 +0000
From: Cool Lee <cool_lee@...eedtech.com>
To: Andrew Jeffery <andrew@...econstruct.com.au>, "adrian.hunter@...el.com"
	<adrian.hunter@...el.com>, "ulf.hansson@...aro.org" <ulf.hansson@...aro.org>,
	"joel@....id.au" <joel@....id.au>, "p.zabel@...gutronix.de"
	<p.zabel@...gutronix.de>, "linux-aspeed@...ts.ozlabs.org"
	<linux-aspeed@...ts.ozlabs.org>, "openbmc@...ts.ozlabs.org"
	<openbmc@...ts.ozlabs.org>, "linux-mmc@...r.kernel.org"
	<linux-mmc@...r.kernel.org>, "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, BMC-SW <BMC-SW@...eedtech.com>
Subject: RE: [PATCH 5/8] mmc: sdhci-of-aspeed: Fix null pointer


> > Platform data might be null.
> 
> Currently it can't be:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers
> /mmc/host/sdhci-of-aspeed.c?h=v6.16-rc2#n375
> 
> Are there future circumstances where it may be NULL?
> 
> I'm all for reducing the reasoning from global to local, but I think some
> discussion in the commit message would be good.
Ok, you're right. I'll remove this.
I'm just considering when adding a new platform AST2700 which doesn't have a data, but missed there is a pre-check here.

> 
> >
> > Signed-off-by: Cool Lee <cool_lee@...eedtech.com>
> > ---
> >  drivers/mmc/host/sdhci-of-aspeed.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci-of-aspeed.c
> > b/drivers/mmc/host/sdhci-of-aspeed.c
> > index 2bdd93a3f91f..22dde915e51b 100644
> > --- a/drivers/mmc/host/sdhci-of-aspeed.c
> > +++ b/drivers/mmc/host/sdhci-of-aspeed.c
> > @@ -241,7 +241,7 @@ static void aspeed_sdhci_set_clock(struct
> > sdhci_host *host, unsigned int clock)
> >         struct sdhci_pltfm_host *pltfm_host;
> >         unsigned long parent, bus;
> >         struct aspeed_sdhci *sdhci;
> > -       int div;
> > +       int div = 1;
> >         u16 clk;
> >
> >         pltfm_host = sdhci_priv(host); @@ -257,6 +257,9 @@ static
> void
> > aspeed_sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
> >         if (WARN_ON(clock > host->max_clk))
> >                 clock = host->max_clk;
> >
> > +       if (sdhci->pdata)
> 
> Given this shouldn't be the case, perhaps precede it with a
> WARN_ONCE(!sdhci->pdata)?
Agreed.

> 
> Andrew
> 
> > +               div = sdhci->pdata->clk_div_start;
> > +
> >         /*
> >          * Regarding the AST2600:
> >          *
> > @@ -273,7 +276,7 @@ static void aspeed_sdhci_set_clock(struct
> > sdhci_host *host, unsigned int clock)
> >          * supporting the value 0 in (EMMC12C[7:6], EMMC12C[15:8]),
> > and capture
> >          * the 0-value capability in clk_div_start.
> >          */
> > -       for (div = sdhci->pdata->clk_div_start; div < 256; div *= 2) {
> > +       for (; div < 256; div *= 2) {
> >                 bus = parent / div;
> >                 if (bus <= clock)
> >                         break;