[U-Boot] [PATCH 1/7] mmc: atmel_sdhci: fix uninitialized pointer deref on probe

[U-Boot] [PATCH 1/7] mmc: atmel_sdhci: fix uninitialized pointer deref on probe

[Peng Fan]

Commit 3d296365e4e8 ("mmc: sdhci: Add support for
sdhci-caps-mask") sdhci_setup_cfg() expects a valid sdhci_host mmc
field.

Fixes: 3d296365e4e8 ("mmc: sdhci: Add support for sdhci-caps-mask")
Cc: Faiz Abbas <faiz_abbas@ti.com>
Cc: Wenyou Yang <wenyou.yang@atmel.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/mmc/atmel_sdhci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/atmel_sdhci.c b/drivers/mmc/atmel_sdhci.c
index 4be47ba75e..d930ed8da0 100644
--- a/drivers/mmc/atmel_sdhci.c
+++ b/drivers/mmc/atmel_sdhci.c
@@ -88,13 +88,13 @@ static int atmel_sdhci_probe(struct udevice *dev)
 		return -EINVAL;
 
 	host->max_clk = max_clk;
+	host->mmc = &plat->mmc;
+	host->mmc->dev = dev;
 
 	ret = sdhci_setup_cfg(&plat->cfg, host, 0, ATMEL_SDHC_MIN_FREQ);
 	if (ret)
 		return ret;
 
-	host->mmc = &plat->mmc;
-	host->mmc->dev = dev;
 	host->mmc->priv = host;
 	upriv->mmc = host->mmc;
 
-- 
2.16.4

[U-Boot] [PATCH 2/7] mmc: bcm: fix uninitialized pointer deref on probe

[Peng Fan]

Commit 3d296365e4e8 ("mmc: sdhci: Add support for
sdhci-caps-mask") sdhci_setup_cfg() expects a valid sdhci_host mmc
field.

Fixes: 3d296365e4e8 ("mmc: sdhci: Add support for sdhci-caps-mask")
Cc: Faiz Abbas <faiz_abbas@ti.com>
Cc: Matthias Brugger <mbrugger@suse.com>
Cc: Thomas Fitzsimmons <fitzsim@fitzsim.org>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/mmc/bcm2835_sdhci.c | 4 +++-
 drivers/mmc/bcmstb_sdhci.c  | 3 ++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/bcm2835_sdhci.c b/drivers/mmc/bcm2835_sdhci.c
index 08bddd410e..bf3304c4dc 100644
--- a/drivers/mmc/bcm2835_sdhci.c
+++ b/drivers/mmc/bcm2835_sdhci.c
@@ -214,6 +214,9 @@ static int bcm2835_sdhci_probe(struct udevice *dev)
 	host->voltages = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195;
 	host->ops = &bcm2835_ops;
 
+	host->mmc = &plat->mmc;
+	host->mmc->dev = dev;
+
 	ret = sdhci_setup_cfg(&plat->cfg, host, emmc_freq, MIN_FREQ);
 	if (ret) {
 		debug("%s: Failed to setup SDHCI (err=%d)\n", __func__, ret);
@@ -221,7 +224,6 @@ static int bcm2835_sdhci_probe(struct udevice *dev)
 	}
 
 	upriv->mmc = &plat->mmc;
-	host->mmc = &plat->mmc;
 	host->mmc->priv = host;
 
 	return sdhci_probe(dev);
diff --git a/drivers/mmc/bcmstb_sdhci.c b/drivers/mmc/bcmstb_sdhci.c
index eef46f3af1..c14f8289e6 100644
--- a/drivers/mmc/bcmstb_sdhci.c
+++ b/drivers/mmc/bcmstb_sdhci.c
@@ -73,6 +73,8 @@ static int sdhci_bcmstb_probe(struct udevice *dev)
 	if (ret)
 		return ret;
 
+	host->mmc = &plat->mmc;
+	host->mmc->dev = dev;
 	ret = sdhci_setup_cfg(&plat->cfg, host,
 			      BCMSTB_SDHCI_MAXIMUM_CLOCK_FREQUENCY,
 			      BCMSTB_SDHCI_MINIMUM_CLOCK_FREQUENCY);
@@ -80,7 +82,6 @@ static int sdhci_bcmstb_probe(struct udevice *dev)
 		return ret;
 
 	upriv->mmc = &plat->mmc;
-	host->mmc = &plat->mmc;
 	host->mmc->priv = host;
 
 	return sdhci_probe(dev);
-- 
2.16.4

[U-Boot] [PATCH 3/7] mmc: msm_sdhci: fix uninitialized pointer deref on probe

[Peng Fan]

Commit 3d296365e4e8 ("mmc: sdhci: Add support for
sdhci-caps-mask") sdhci_setup_cfg() expects a valid sdhci_host mmc
field.

Fixes: 3d296365e4e8 ("mmc: sdhci: Add support for sdhci-caps-mask")
Cc: Faiz Abbas <faiz_abbas@ti.com>
Cc: Ramon Fried <rfried.dev@gmail.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/mmc/msm_sdhci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/msm_sdhci.c b/drivers/mmc/msm_sdhci.c
index 51f9e0ec05..cae42ec4ac 100644
--- a/drivers/mmc/msm_sdhci.c
+++ b/drivers/mmc/msm_sdhci.c
@@ -141,12 +141,12 @@ static int msm_sdc_probe(struct udevice *dev)
 		writel(caps, host->ioaddr + SDHCI_VENDOR_SPEC_CAPABILITIES0);
 	}
 
-	ret = sdhci_setup_cfg(&plat->cfg, host, 0, 0);
 	host->mmc = &plat->mmc;
+	host->mmc->dev = dev;
+	ret = sdhci_setup_cfg(&plat->cfg, host, 0, 0);
 	if (ret)
 		return ret;
 	host->mmc->priv = &prv->host;
-	host->mmc->dev = dev;
 	upriv->mmc = host->mmc;
 
 	return sdhci_probe(dev);
-- 
2.16.4

[U-Boot] [PATCH 4/7] mmc: pci: fix uninitialized pointer deref on probe

[Peng Fan]

Commit 3d296365e4e8 ("mmc: sdhci: Add support for
sdhci-caps-mask") sdhci_setup_cfg() expects a valid sdhci_host mmc
field.

Fixes: 3d296365e4e8 ("mmc: sdhci: Add support for sdhci-caps-mask")
Cc: Faiz Abbas <faiz_abbas@ti.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Bin Meng <bmeng.cn@gmail.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/mmc/pci_mmc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/pci_mmc.c b/drivers/mmc/pci_mmc.c
index 182d41637f..404264a697 100644
--- a/drivers/mmc/pci_mmc.c
+++ b/drivers/mmc/pci_mmc.c
@@ -33,12 +33,12 @@ static int pci_mmc_probe(struct udevice *dev)
 	host->ioaddr = (void *)dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0,
 					      PCI_REGION_MEM);
 	host->name = dev->name;
+	host->mmc = &plat->mmc;
+	host->mmc->dev = dev;
 	ret = sdhci_setup_cfg(&plat->cfg, host, 0, 0);
 	if (ret)
 		return ret;
-	host->mmc = &plat->mmc;
 	host->mmc->priv = &priv->host;
-	host->mmc->dev = dev;
 	upriv->mmc = host->mmc;
 
 	return sdhci_probe(dev);
-- 
2.16.4

[U-Boot] [PATCH 5/7] mmc: s5p: fix uninitialized pointer deref on probe

[Peng Fan]

Commit 3d296365e4e8 ("mmc: sdhci: Add support for
sdhci-caps-mask") sdhci_setup_cfg() expects a valid sdhci_host mmc
field.

Fixes: 3d296365e4e8 ("mmc: sdhci: Add support for sdhci-caps-mask")
Cc: Faiz Abbas <faiz_abbas@ti.com>
Cc: Jaehoon Chung <jh80.chung@samsung.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/mmc/s5p_sdhci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/s5p_sdhci.c b/drivers/mmc/s5p_sdhci.c
index 9dd0b865eb..53efa968cf 100644
--- a/drivers/mmc/s5p_sdhci.c
+++ b/drivers/mmc/s5p_sdhci.c
@@ -204,13 +204,13 @@ static int s5p_sdhci_probe(struct udevice *dev)
 	if (ret)
 		return ret;
 
+	host->mmc = &plat->mmc;
+	host->mmc->dev = dev;
 	ret = sdhci_setup_cfg(&plat->cfg, host, 0, 400000);
 	if (ret)
 		return ret;
 
-	host->mmc = &plat->mmc;
 	host->mmc->priv = host;
-	host->mmc->dev = dev;
 	upriv->mmc = host->mmc;
 
 	return sdhci_probe(dev);
-- 
2.16.4

[U-Boot] [PATCH 6/7] mmc: sdhci-cadence: fix uninitialized pointer deref on probe

[Peng Fan]

Commit 3d296365e4e8 ("mmc: sdhci: Add support for
sdhci-caps-mask") sdhci_setup_cfg() expects a valid sdhci_host mmc
field.

Fixes: 3d296365e4e8 ("mmc: sdhci: Add support for sdhci-caps-mask")
Cc: Faiz Abbas <faiz_abbas@ti.com>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/mmc/sdhci-cadence.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/sdhci-cadence.c b/drivers/mmc/sdhci-cadence.c
index 4f9338f733..4736263bf2 100644
--- a/drivers/mmc/sdhci-cadence.c
+++ b/drivers/mmc/sdhci-cadence.c
@@ -269,12 +269,13 @@ static int sdhci_cdns_probe(struct udevice *dev)
 	if (ret)
 		return ret;
 
+	host->mmc = &plat->mmc;
+	host->mmc->dev = dev;
 	ret = sdhci_setup_cfg(&plat->cfg, host, 0, 0);
 	if (ret)
 		return ret;
 
 	upriv->mmc = &plat->mmc;
-	host->mmc = &plat->mmc;
 	host->mmc->priv = host;
 
 	return sdhci_probe(dev);
-- 
2.16.4

[U-Boot] [PATCH 7/7] mmc: tangier_sdhci: fix uninitialized pointer deref on probe

[Peng Fan]

Commit 3d296365e4e8 ("mmc: sdhci: Add support for
sdhci-caps-mask") sdhci_setup_cfg() expects a valid sdhci_host mmc
field.

Fixes: 3d296365e4e8 ("mmc: sdhci: Add support for sdhci-caps-mask")
Cc: Faiz Abbas <faiz_abbas@ti.com>
Cc: Vincent Tinelli <vincent.tinelli@intel.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/mmc/tangier_sdhci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/tangier_sdhci.c b/drivers/mmc/tangier_sdhci.c
index 4c33356b9f..0d6e5d6246 100644
--- a/drivers/mmc/tangier_sdhci.c
+++ b/drivers/mmc/tangier_sdhci.c
@@ -51,13 +51,14 @@ static int sdhci_tangier_probe(struct udevice *dev)
 	/* MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195 */
 	host->voltages = MMC_VDD_165_195;
 
+	host->mmc = &plat->mmc;
+	host->mmc->dev = dev;
 	ret = sdhci_setup_cfg(&plat->cfg, host, SDHCI_TANGIER_FMAX,
 			SDHCI_TANGIER_FMIN);
 	if (ret)
 		return ret;
 
 	upriv->mmc = &plat->mmc;
-	host->mmc = &plat->mmc;
 	host->mmc->priv = host;
 
 	return sdhci_probe(dev);
-- 
2.16.4

[U-Boot] [PATCH 6/7] mmc: sdhci-cadence: fix uninitialized pointer deref on probe

[Masahiro Yamada]

On Tue, Aug 6, 2019 at 11:50 AM Peng Fan <peng.fan@nxp.com> wrote:
>
> Commit 3d296365e4e8 ("mmc: sdhci: Add support for
> sdhci-caps-mask") sdhci_setup_cfg() expects a valid sdhci_host mmc
> field.
>
> Fixes: 3d296365e4e8 ("mmc: sdhci: Add support for sdhci-caps-mask")
> Cc: Faiz Abbas <faiz_abbas@ti.com>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>


Acked-by: Masahiro Yamada <yamada.masahiro@socionext.com>


-- 
Best Regards
Masahiro Yamada

[U-Boot] [PATCH 1/7] mmc: atmel_sdhci: fix uninitialized pointer deref on probe

[Eugen.Hristev at microchip.com]

Tested-by: Eugen Hristev <eugen.hristev@microchip.com>

This has to go as soon as possible into the tree.
The at91 boards do not boot without this. The commit checking for 
sdhci-caps-mask introduces a crash at this point.

I can take it through atmel tree if needed.

Thanks,
Eugen

On 06.08.2019 05:47, Peng Fan wrote:

> 
> Commit 3d296365e4e8 ("mmc: sdhci: Add support for
> sdhci-caps-mask") sdhci_setup_cfg() expects a valid sdhci_host mmc
> field.
> 
> Fixes: 3d296365e4e8 ("mmc: sdhci: Add support for sdhci-caps-mask")
> Cc: Faiz Abbas <faiz_abbas@ti.com>
> Cc: Wenyou Yang <wenyou.yang@atmel.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>   drivers/mmc/atmel_sdhci.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/atmel_sdhci.c b/drivers/mmc/atmel_sdhci.c
> index 4be47ba75e..d930ed8da0 100644
> --- a/drivers/mmc/atmel_sdhci.c
> +++ b/drivers/mmc/atmel_sdhci.c
> @@ -88,13 +88,13 @@ static int atmel_sdhci_probe(struct udevice *dev)
>   		return -EINVAL;
>   
>   	host->max_clk = max_clk;
> +	host->mmc = &plat->mmc;
> +	host->mmc->dev = dev;
>   
>   	ret = sdhci_setup_cfg(&plat->cfg, host, 0, ATMEL_SDHC_MIN_FREQ);
>   	if (ret)
>   		return ret;
>   
> -	host->mmc = &plat->mmc;
> -	host->mmc->dev = dev;
>   	host->mmc->priv = host;
>   	upriv->mmc = host->mmc;
>   
> 

[U-Boot] [PATCH 1/7] mmc: atmel_sdhci: fix uninitialized pointer deref on probe

[Peng Fan]

> Subject: Re: [U-Boot] [PATCH 1/7] mmc: atmel_sdhci: fix uninitialized pointer
> deref on probe
> 
> Tested-by: Eugen Hristev <eugen.hristev@microchip.com>
> 
> This has to go as soon as possible into the tree.
> The at91 boards do not boot without this. The commit checking for
> sdhci-caps-mask introduces a crash at this point.
> 
> I can take it through atmel tree if needed.


I need wait R-b and A-b for other patches in the patchset,
Then PR to Tom. So please take this patch if hurry, and mark
patchwork as accepted.

Thanks,
Peng.

> 
> Thanks,
> Eugen
> 
> On 06.08.2019 05:47, Peng Fan wrote:
> 
> >
> > Commit 3d296365e4e8 ("mmc: sdhci: Add support for
> > sdhci-caps-mask") sdhci_setup_cfg() expects a valid sdhci_host mmc
> > field.
> >
> > Fixes: 3d296365e4e8 ("mmc: sdhci: Add support for sdhci-caps-mask")
> > Cc: Faiz Abbas <faiz_abbas@ti.com>
> > Cc: Wenyou Yang <wenyou.yang@atmel.com>
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >   drivers/mmc/atmel_sdhci.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mmc/atmel_sdhci.c b/drivers/mmc/atmel_sdhci.c
> > index 4be47ba75e..d930ed8da0 100644
> > --- a/drivers/mmc/atmel_sdhci.c
> > +++ b/drivers/mmc/atmel_sdhci.c
> > @@ -88,13 +88,13 @@ static int atmel_sdhci_probe(struct udevice *dev)
> >   		return -EINVAL;
> >
> >   	host->max_clk = max_clk;
> > +	host->mmc = &plat->mmc;
> > +	host->mmc->dev = dev;
> >
> >   	ret = sdhci_setup_cfg(&plat->cfg, host, 0,
> ATMEL_SDHC_MIN_FREQ);
> >   	if (ret)
> >   		return ret;
> >
> > -	host->mmc = &plat->mmc;
> > -	host->mmc->dev = dev;
> >   	host->mmc->priv = host;
> >   	upriv->mmc = host->mmc;
> >
> >

[U-Boot] [PATCH 1/7] mmc: atmel_sdhci: fix uninitialized pointer deref on probe

[Eugen.Hristev at microchip.com]

On 07.08.2019 13:04, Peng Fan wrote:

> 
>> Subject: Re: [U-Boot] [PATCH 1/7] mmc: atmel_sdhci: fix uninitialized pointer
>> deref on probe
>>
>> Tested-by: Eugen Hristev <eugen.hristev@microchip.com>
>>
>> This has to go as soon as possible into the tree.
>> The at91 boards do not boot without this. The commit checking for
>> sdhci-caps-mask introduces a crash at this point.
>>
>> I can take it through atmel tree if needed.
> 
> 
> I need wait R-b and A-b for other patches in the patchset,
> Then PR to Tom. So please take this patch if hurry, and mark
> patchwork as accepted.
> 

As it looks to me all the other platforms have the same issue. So they 
are just as urgent. In this case I assume it won't take long for all of 
them to be taken, so, it's fine.

Just one question, why did you not move all the 4 initialization lines 
before the sdhci_setup_cfg ? Other drivers initialize all 4 before the call.

Eugen


> Thanks,
> Peng.
> 
>>
>> Thanks,
>> Eugen
>>
>> On 06.08.2019 05:47, Peng Fan wrote:
>>
>>>
>>> Commit 3d296365e4e8 ("mmc: sdhci: Add support for
>>> sdhci-caps-mask") sdhci_setup_cfg() expects a valid sdhci_host mmc
>>> field.
>>>
>>> Fixes: 3d296365e4e8 ("mmc: sdhci: Add support for sdhci-caps-mask")
>>> Cc: Faiz Abbas <faiz_abbas@ti.com>
>>> Cc: Wenyou Yang <wenyou.yang@atmel.com>
>>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>>> ---
>>>    drivers/mmc/atmel_sdhci.c | 4 ++--
>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/mmc/atmel_sdhci.c b/drivers/mmc/atmel_sdhci.c
>>> index 4be47ba75e..d930ed8da0 100644
>>> --- a/drivers/mmc/atmel_sdhci.c
>>> +++ b/drivers/mmc/atmel_sdhci.c
>>> @@ -88,13 +88,13 @@ static int atmel_sdhci_probe(struct udevice *dev)
>>>    		return -EINVAL;
>>>
>>>    	host->max_clk = max_clk;
>>> +	host->mmc = &plat->mmc;
>>> +	host->mmc->dev = dev;
>>>
>>>    	ret = sdhci_setup_cfg(&plat->cfg, host, 0,
>> ATMEL_SDHC_MIN_FREQ);
>>>    	if (ret)
>>>    		return ret;
>>>
>>> -	host->mmc = &plat->mmc;
>>> -	host->mmc->dev = dev;
>>>    	host->mmc->priv = host;
>>>    	upriv->mmc = host->mmc;
>>>
>>>

[U-Boot] [PATCH 1/7] mmc: atmel_sdhci: fix uninitialized pointer deref on probe

[Peng Fan]

> Subject: Re: [U-Boot] [PATCH 1/7] mmc: atmel_sdhci: fix uninitialized pointer
> deref on probe
> 
> 
> 
> On 07.08.2019 13:04, Peng Fan wrote:
> 
> >
> >> Subject: Re: [U-Boot] [PATCH 1/7] mmc: atmel_sdhci: fix uninitialized
> >> pointer deref on probe
> >>
> >> Tested-by: Eugen Hristev <eugen.hristev@microchip.com>
> >>
> >> This has to go as soon as possible into the tree.
> >> The at91 boards do not boot without this. The commit checking for
> >> sdhci-caps-mask introduces a crash at this point.
> >>
> >> I can take it through atmel tree if needed.
> >
> >
> > I need wait R-b and A-b for other patches in the patchset, Then PR to
> > Tom. So please take this patch if hurry, and mark patchwork as
> > accepted.
> >
> 
> As it looks to me all the other platforms have the same issue. So they are just
> as urgent. In this case I assume it won't take long for all of them to be taken,
> so, it's fine.
> 
> Just one question, why did you not move all the 4 initialization lines before the
> sdhci_setup_cfg ? Other drivers initialize all 4 before the call.

Only dev is required to address the issue.

Regards,
Peng.

> 
> Eugen
> 
> 
> > Thanks,
> > Peng.
> >
> >>
> >> Thanks,
> >> Eugen
> >>
> >> On 06.08.2019 05:47, Peng Fan wrote:
> >>
> >>>
> >>> Commit 3d296365e4e8 ("mmc: sdhci: Add support for
> >>> sdhci-caps-mask") sdhci_setup_cfg() expects a valid sdhci_host mmc
> >>> field.
> >>>
> >>> Fixes: 3d296365e4e8 ("mmc: sdhci: Add support for sdhci-caps-mask")
> >>> Cc: Faiz Abbas <faiz_abbas@ti.com>
> >>> Cc: Wenyou Yang <wenyou.yang@atmel.com>
> >>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> >>> ---
> >>>    drivers/mmc/atmel_sdhci.c | 4 ++--
> >>>    1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/mmc/atmel_sdhci.c b/drivers/mmc/atmel_sdhci.c
> >>> index 4be47ba75e..d930ed8da0 100644
> >>> --- a/drivers/mmc/atmel_sdhci.c
> >>> +++ b/drivers/mmc/atmel_sdhci.c
> >>> @@ -88,13 +88,13 @@ static int atmel_sdhci_probe(struct udevice *dev)
> >>>    		return -EINVAL;
> >>>
> >>>    	host->max_clk = max_clk;
> >>> +	host->mmc = &plat->mmc;
> >>> +	host->mmc->dev = dev;
> >>>
> >>>    	ret = sdhci_setup_cfg(&plat->cfg, host, 0,
> >> ATMEL_SDHC_MIN_FREQ);
> >>>    	if (ret)
> >>>    		return ret;
> >>>
> >>> -	host->mmc = &plat->mmc;
> >>> -	host->mmc->dev = dev;
> >>>    	host->mmc->priv = host;
> >>>    	upriv->mmc = host->mmc;
> >>>
> >>>

[U-Boot] [PATCH 1/7] mmc: atmel_sdhci: fix uninitialized pointer deref on probe

[Eugen.Hristev at microchip.com]

On 08.08.2019 04:47, Peng Fan wrote:

>> Subject: Re: [U-Boot] [PATCH 1/7] mmc: atmel_sdhci: fix uninitialized pointer
>> deref on probe
>>
>>
>>
>> On 07.08.2019 13:04, Peng Fan wrote:
>>
>>>
>>>> Subject: Re: [U-Boot] [PATCH 1/7] mmc: atmel_sdhci: fix uninitialized
>>>> pointer deref on probe
>>>>
>>>> Tested-by: Eugen Hristev <eugen.hristev@microchip.com>
>>>>
>>>> This has to go as soon as possible into the tree.
>>>> The at91 boards do not boot without this. The commit checking for
>>>> sdhci-caps-mask introduces a crash at this point.
>>>>
>>>> I can take it through atmel tree if needed.
>>>
>>>
>>> I need wait R-b and A-b for other patches in the patchset, Then PR to
>>> Tom. So please take this patch if hurry, and mark patchwork as
>>> accepted.
>>>
>>
>> As it looks to me all the other platforms have the same issue. So they are just
>> as urgent. In this case I assume it won't take long for all of them to be taken,
>> so, it's fine.
>>
>> Just one question, why did you not move all the 4 initialization lines before the
>> sdhci_setup_cfg ? Other drivers initialize all 4 before the call.
> 
> Only dev is required to address the issue.
> 

True. But other drivers perform all 4 initializations. In the future the 
sdhci core might again make wrong assumptions about them, and we might 
get another crash?
Why not move all the 4 lines now ? It looks like they are just 
backpointers. So I would say the initialization of the pointers is 
incomplete before the sdhci_setup_cfg call, would be better to do it 
completely

Eugen

> Regards,
> Peng.
> 
>>
>> Eugen
>>
>>
>>> Thanks,
>>> Peng.
>>>
>>>>
>>>> Thanks,
>>>> Eugen
>>>>
>>>> On 06.08.2019 05:47, Peng Fan wrote:
>>>>
>>>>>
>>>>> Commit 3d296365e4e8 ("mmc: sdhci: Add support for
>>>>> sdhci-caps-mask") sdhci_setup_cfg() expects a valid sdhci_host mmc
>>>>> field.
>>>>>
>>>>> Fixes: 3d296365e4e8 ("mmc: sdhci: Add support for sdhci-caps-mask")
>>>>> Cc: Faiz Abbas <faiz_abbas@ti.com>
>>>>> Cc: Wenyou Yang <wenyou.yang@atmel.com>
>>>>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>>>>> ---
>>>>>     drivers/mmc/atmel_sdhci.c | 4 ++--
>>>>>     1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/mmc/atmel_sdhci.c b/drivers/mmc/atmel_sdhci.c
>>>>> index 4be47ba75e..d930ed8da0 100644
>>>>> --- a/drivers/mmc/atmel_sdhci.c
>>>>> +++ b/drivers/mmc/atmel_sdhci.c
>>>>> @@ -88,13 +88,13 @@ static int atmel_sdhci_probe(struct udevice *dev)
>>>>>     		return -EINVAL;
>>>>>
>>>>>     	host->max_clk = max_clk;
>>>>> +	host->mmc = &plat->mmc;
>>>>> +	host->mmc->dev = dev;
>>>>>
>>>>>     	ret = sdhci_setup_cfg(&plat->cfg, host, 0,
>>>> ATMEL_SDHC_MIN_FREQ);
>>>>>     	if (ret)
>>>>>     		return ret;
>>>>>
>>>>> -	host->mmc = &plat->mmc;
>>>>> -	host->mmc->dev = dev;
>>>>>     	host->mmc->priv = host;
>>>>>     	upriv->mmc = host->mmc;
>>>>>
>>>>>

[U-Boot] [PATCH 1/7] mmc: atmel_sdhci: fix uninitialized pointer deref on probe

[Peng Fan]

> 
> >> Subject: Re: [U-Boot] [PATCH 1/7] mmc: atmel_sdhci: fix uninitialized
> >> pointer deref on probe
> >>
> >>
> >>
> >> On 07.08.2019 13:04, Peng Fan wrote:
> >>
> >>>
> >>>> Subject: Re: [U-Boot] [PATCH 1/7] mmc: atmel_sdhci: fix
> >>>> uninitialized pointer deref on probe
> >>>>
> >>>> Tested-by: Eugen Hristev <eugen.hristev@microchip.com>
> >>>>
> >>>> This has to go as soon as possible into the tree.
> >>>> The at91 boards do not boot without this. The commit checking for
> >>>> sdhci-caps-mask introduces a crash at this point.
> >>>>
> >>>> I can take it through atmel tree if needed.
> >>>
> >>>
> >>> I need wait R-b and A-b for other patches in the patchset, Then PR
> >>> to Tom. So please take this patch if hurry, and mark patchwork as
> >>> accepted.
> >>>
> >>
> >> As it looks to me all the other platforms have the same issue. So
> >> they are just as urgent. In this case I assume it won't take long for
> >> all of them to be taken, so, it's fine.
> >>
> >> Just one question, why did you not move all the 4 initialization
> >> lines before the sdhci_setup_cfg ? Other drivers initialize all 4 before the
> call.
> >
> > Only dev is required to address the issue.
> >
> 
> True. But other drivers perform all 4 initializations. In the future the sdhci core
> might again make wrong assumptions about them, and we might get another
> crash?
> Why not move all the 4 lines now ? It looks like they are just backpointers. So
> I would say the initialization of the pointers is incomplete before the
> sdhci_setup_cfg call, would be better to do it completely

After checking the code, sdhci.c is common code, priv pointer should be not
used there. So no need to move priv here.
and upriv is used by mmc uclass.

The remaining 2 has no chance to be used by sdhci.c.

Regards,
Peng.

> 
> Eugen
> 
> > Regards,
> > Peng.
> >
> >>
> >> Eugen
> >>
> >>
> >>> Thanks,
> >>> Peng.
> >>>
> >>>>
> >>>> Thanks,
> >>>> Eugen
> >>>>
> >>>> On 06.08.2019 05:47, Peng Fan wrote:
> >>>>
> >>>>>
> >>>>> Commit 3d296365e4e8 ("mmc: sdhci: Add support for
> >>>>> sdhci-caps-mask") sdhci_setup_cfg() expects a valid sdhci_host mmc
> >>>>> field.
> >>>>>
> >>>>> Fixes: 3d296365e4e8 ("mmc: sdhci: Add support for
> >>>>> sdhci-caps-mask")
> >>>>> Cc: Faiz Abbas <faiz_abbas@ti.com>
> >>>>> Cc: Wenyou Yang <wenyou.yang@atmel.com>
> >>>>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> >>>>> ---
> >>>>>     drivers/mmc/atmel_sdhci.c | 4 ++--
> >>>>>     1 file changed, 2 insertions(+), 2 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/mmc/atmel_sdhci.c b/drivers/mmc/atmel_sdhci.c
> >>>>> index 4be47ba75e..d930ed8da0 100644
> >>>>> --- a/drivers/mmc/atmel_sdhci.c
> >>>>> +++ b/drivers/mmc/atmel_sdhci.c
> >>>>> @@ -88,13 +88,13 @@ static int atmel_sdhci_probe(struct udevice
> *dev)
> >>>>>     		return -EINVAL;
> >>>>>
> >>>>>     	host->max_clk = max_clk;
> >>>>> +	host->mmc = &plat->mmc;
> >>>>> +	host->mmc->dev = dev;
> >>>>>
> >>>>>     	ret = sdhci_setup_cfg(&plat->cfg, host, 0,
> >>>> ATMEL_SDHC_MIN_FREQ);
> >>>>>     	if (ret)
> >>>>>     		return ret;
> >>>>>
> >>>>> -	host->mmc = &plat->mmc;
> >>>>> -	host->mmc->dev = dev;
> >>>>>     	host->mmc->priv = host;
> >>>>>     	upriv->mmc = host->mmc;
> >>>>>
> >>>>>

[U-Boot] [PATCH 1/7] mmc: atmel_sdhci: fix uninitialized pointer deref on probe

[Peng Fan]

> Subject: [PATCH 1/7] mmc: atmel_sdhci: fix uninitialized pointer deref on
> probe
> 
> Commit 3d296365e4e8 ("mmc: sdhci: Add support for
> sdhci-caps-mask") sdhci_setup_cfg() expects a valid sdhci_host mmc field.
> 
> Fixes: 3d296365e4e8 ("mmc: sdhci: Add support for sdhci-caps-mask")
> Cc: Faiz Abbas <faiz_abbas@ti.com>
> Cc: Wenyou Yang <wenyou.yang@atmel.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/mmc/atmel_sdhci.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/atmel_sdhci.c b/drivers/mmc/atmel_sdhci.c index
> 4be47ba75e..d930ed8da0 100644
> --- a/drivers/mmc/atmel_sdhci.c
> +++ b/drivers/mmc/atmel_sdhci.c
> @@ -88,13 +88,13 @@ static int atmel_sdhci_probe(struct udevice *dev)
>  		return -EINVAL;
> 
>  	host->max_clk = max_clk;
> +	host->mmc = &plat->mmc;
> +	host->mmc->dev = dev;
> 
>  	ret = sdhci_setup_cfg(&plat->cfg, host, 0, ATMEL_SDHC_MIN_FREQ);
>  	if (ret)
>  		return ret;
> 
> -	host->mmc = &plat->mmc;
> -	host->mmc->dev = dev;
>  	host->mmc->priv = host;
>  	upriv->mmc = host->mmc;

Patchset applied to mmc/master.

Thanks,
Peng.

> 
> --
> 2.16.4

[U-Boot] [PATCH 4/7] mmc: pci: fix uninitialized pointer deref on probe

[Simon Glass]

On Mon, 5 Aug 2019 at 20:48, Peng Fan <peng.fan@nxp.com> wrote:
>
> Commit 3d296365e4e8 ("mmc: sdhci: Add support for
> sdhci-caps-mask") sdhci_setup_cfg() expects a valid sdhci_host mmc
> field.
>
> Fixes: 3d296365e4e8 ("mmc: sdhci: Add support for sdhci-caps-mask")
> Cc: Faiz Abbas <faiz_abbas@ti.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Bin Meng <bmeng.cn@gmail.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/mmc/pci_mmc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

>
> diff --git a/drivers/mmc/pci_mmc.c b/drivers/mmc/pci_mmc.c
> index 182d41637f..404264a697 100644
> --- a/drivers/mmc/pci_mmc.c
> +++ b/drivers/mmc/pci_mmc.c
> @@ -33,12 +33,12 @@ static int pci_mmc_probe(struct udevice *dev)
>         host->ioaddr = (void *)dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0,
>                                               PCI_REGION_MEM);
>         host->name = dev->name;
> +       host->mmc = &plat->mmc;
> +       host->mmc->dev = dev;

How about putting through three lines in a common function in
mmc-uclass.c (with args 'host', dev and play) since they appear in a
lot of drivers.

Regards,
Simon