[U-Boot] [PATCH] mmc: sdhci: fix NULL pointer access when host->ops is not set
Until recently, sdhci_ops was used only for overriding IO accessors. (so, host->ops was not set by any drivers except bcm2835_sdhci.c)
Now, we have more optional callbacks, get_cd, set_control_reg, and set_clock. However, the code
if (host->ops->get_cd) host->ops->get_cd(host);
... expects host->ops is set for all drivers.
Commit 5e96217f0434 ("mmc: pic32_sdhci: move the code to pic32_sdhci.c") and commit 62226b68631b ("mmc: sdhci: move the callback function into sdhci_ops") added sdhci_ops for pic32_sdhci.c and s5p_sdhci.c, but the other drivers still do not (need not) set host->ops because all callbacks in sdhci_ops are optional.
host->ops must be checked to avoid the system crash caused by NULL pointer access.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com ---
drivers/mmc/sdhci.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index 3a1f4f7..5b404ff 100644 --- a/drivers/mmc/sdhci.c +++ b/drivers/mmc/sdhci.c @@ -359,7 +359,7 @@ static int sdhci_set_clock(struct mmc *mmc, unsigned int clock) div >>= 1; }
- if (host->ops->set_clock) + if (host->ops && host->ops->set_clock) host->ops->set_clock(host, div);
clk |= (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT; @@ -427,7 +427,7 @@ static int sdhci_set_ios(struct mmc *mmc) u32 ctrl; struct sdhci_host *host = mmc->priv;
- if (host->ops->set_control_reg) + if (host->ops && host->ops->set_control_reg) host->ops->set_control_reg(host);
if (mmc->clock != host->clock) @@ -480,7 +480,7 @@ static int sdhci_init(struct mmc *mmc)
sdhci_set_power(host, fls(mmc->cfg->voltages) - 1);
- if (host->ops->get_cd) + if (host->ops && host->ops->get_cd) host->ops->get_cd(host);
/* Enable only interrupts served by the SD controller */
Tom did not pull MMC updates issued just before v2017.01 and he was right.
2017-01-13 11:51 GMT+09:00 Masahiro Yamada yamada.masahiro@socionext.com:
Until recently, sdhci_ops was used only for overriding IO accessors. (so, host->ops was not set by any drivers except bcm2835_sdhci.c)
Now, we have more optional callbacks, get_cd, set_control_reg, and set_clock. However, the code
if (host->ops->get_cd) host->ops->get_cd(host);... expects host->ops is set for all drivers.
Commit 5e96217f0434 ("mmc: pic32_sdhci: move the code to pic32_sdhci.c") and commit 62226b68631b ("mmc: sdhci: move the callback function into sdhci_ops") added sdhci_ops for pic32_sdhci.c and s5p_sdhci.c, but the other drivers still do not (need not) set host->ops because all callbacks in sdhci_ops are optional.
host->ops must be checked to avoid the system crash caused by NULL pointer access.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com
drivers/mmc/sdhci.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index 3a1f4f7..5b404ff 100644 --- a/drivers/mmc/sdhci.c +++ b/drivers/mmc/sdhci.c @@ -359,7 +359,7 @@ static int sdhci_set_clock(struct mmc *mmc, unsigned int clock) div >>= 1; }
if (host->ops->set_clock)
if (host->ops && host->ops->set_clock) host->ops->set_clock(host, div); clk |= (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;@@ -427,7 +427,7 @@ static int sdhci_set_ios(struct mmc *mmc) u32 ctrl; struct sdhci_host *host = mmc->priv;
if (host->ops->set_control_reg)
if (host->ops && host->ops->set_control_reg) host->ops->set_control_reg(host); if (mmc->clock != host->clock)@@ -480,7 +480,7 @@ static int sdhci_init(struct mmc *mmc)
sdhci_set_power(host, fls(mmc->cfg->voltages) - 1);
if (host->ops->get_cd)
if (host->ops && host->ops->get_cd) host->ops->get_cd(host); /* Enable only interrupts served by the SD controller */-- 2.7.4
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Hi,
On 01/13/2017 11:51 AM, Masahiro Yamada wrote:
Until recently, sdhci_ops was used only for overriding IO accessors. (so, host->ops was not set by any drivers except bcm2835_sdhci.c)
Now, we have more optional callbacks, get_cd, set_control_reg, and set_clock. However, the code
if (host->ops->get_cd) host->ops->get_cd(host);... expects host->ops is set for all drivers.
Commit 5e96217f0434 ("mmc: pic32_sdhci: move the code to pic32_sdhci.c") and commit 62226b68631b ("mmc: sdhci: move the callback function into sdhci_ops") added sdhci_ops for pic32_sdhci.c and s5p_sdhci.c, but the other drivers still do not (need not) set host->ops because all callbacks in sdhci_ops are optional.
host->ops must be checked to avoid the system crash caused by NULL pointer access.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com
Thanks a lot! Applied on u-boot-mmc.
Best Regards, Jaehoon Chung
drivers/mmc/sdhci.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index 3a1f4f7..5b404ff 100644 --- a/drivers/mmc/sdhci.c +++ b/drivers/mmc/sdhci.c @@ -359,7 +359,7 @@ static int sdhci_set_clock(struct mmc *mmc, unsigned int clock) div >>= 1; }
- if (host->ops->set_clock)
if (host->ops && host->ops->set_clock) host->ops->set_clock(host, div);
clk |= (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
@@ -427,7 +427,7 @@ static int sdhci_set_ios(struct mmc *mmc) u32 ctrl; struct sdhci_host *host = mmc->priv;
- if (host->ops->set_control_reg)
if (host->ops && host->ops->set_control_reg) host->ops->set_control_reg(host);
if (mmc->clock != host->clock)
@@ -480,7 +480,7 @@ static int sdhci_init(struct mmc *mmc)
sdhci_set_power(host, fls(mmc->cfg->voltages) - 1);
- if (host->ops->get_cd)
if (host->ops && host->ops->get_cd) host->ops->get_cd(host);
/* Enable only interrupts served by the SD controller */
participants (2)
-
Jaehoon Chung -
Masahiro Yamada