[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 at 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 at ti.com>
>>>>> Cc: Wenyou Yang <wenyou.yang at atmel.com>
>>>>> Signed-off-by: Peng Fan <peng.fan at 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;
>>>>>
>>>>>