[PATCH] mmc: sunxi: Prevent against null dereference for vmmc

Subsystems: multimedia card (mmc), secure digital (sd) and sdio subsystem, the rest

DORMANTno replies

2 messages, 2 authors, 2016-10-19 · open the first message on its own page

[PATCH] mmc: sunxi: Prevent against null dereference for vmmc

From: Maxime Ripard <hidden>
Date: 2016-10-19 15:04:56

VMMC is an optional regulator, which means that mmc_regulator_get_supply
will only return an error in case of a deferred probe, but not when the
regulator is not set in the DT.

However, the sunxi driver assumes that VMMC is always there, and doesn't
check the value of the regulator pointer before using it, which obviously
leads to a (close to) null pointer dereference.

Add proper checks to prevent that.

Signed-off-by: Maxime Ripard <redacted>
---
 drivers/mmc/host/sunxi-mmc.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index c0a5c676d0e8..45a051e7d650 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -822,10 +822,16 @@ static void sunxi_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 		break;
 
 	case MMC_POWER_UP:
-		host->ferror = mmc_regulator_set_ocr(mmc, mmc->supply.vmmc,
-						     ios->vdd);
-		if (host->ferror)
-			return;
+		if (!IS_ERR(mmc->supply.vmmc)) {
+			host->ferror = mmc_regulator_set_ocr(mmc,
+							     mmc->supply.vmmc,
+							     ios->vdd);
+			if (host->ferror) {
+				dev_err(mmc_dev(mmc),
+					"failed to enable vmmc\n");
+				return;
+			}
+		}
 
 		if (!IS_ERR(mmc->supply.vqmmc)) {
 			host->ferror = regulator_enable(mmc->supply.vqmmc);
@@ -847,7 +853,9 @@ static void sunxi_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	case MMC_POWER_OFF:
 		dev_dbg(mmc_dev(mmc), "power off!\n");
 		sunxi_mmc_reset_host(host);
-		mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0);
+		if (!IS_ERR(mmc->supply.vmmc))
+			mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0);
+
 		if (!IS_ERR(mmc->supply.vqmmc) && host->vqmmc_enabled)
 			regulator_disable(mmc->supply.vqmmc);
 		host->vqmmc_enabled = false;
-- 
2.9.3

Re: [PATCH] mmc: sunxi: Prevent against null dereference for vmmc

From: Ulf Hansson <hidden>
Date: 2016-10-19 14:23:48

On 19 October 2016 at 14:36, Maxime Ripard
[off-list ref] wrote:
quoted hunk
VMMC is an optional regulator, which means that mmc_regulator_get_supply
will only return an error in case of a deferred probe, but not when the
regulator is not set in the DT.

However, the sunxi driver assumes that VMMC is always there, and doesn't
check the value of the regulator pointer before using it, which obviously
leads to a (close to) null pointer dereference.

Add proper checks to prevent that.

Signed-off-by: Maxime Ripard <redacted>
---
 drivers/mmc/host/sunxi-mmc.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index c0a5c676d0e8..45a051e7d650 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -822,10 +822,16 @@ static void sunxi_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
                break;

        case MMC_POWER_UP:
-               host->ferror = mmc_regulator_set_ocr(mmc, mmc->supply.vmmc,
-                                                    ios->vdd);
-               if (host->ferror)
-                       return;
+               if (!IS_ERR(mmc->supply.vmmc)) {
+                       host->ferror = mmc_regulator_set_ocr(mmc,
+                                                            mmc->supply.vmmc,
+                                                            ios->vdd);
+                       if (host->ferror) {
+                               dev_err(mmc_dev(mmc),
+                                       "failed to enable vmmc\n");
The print here is already taken care of by mmc_regulator_set_ocr()
quoted hunk
+                               return;
+                       }
+               }

                if (!IS_ERR(mmc->supply.vqmmc)) {
                        host->ferror = regulator_enable(mmc->supply.vqmmc);
@@ -847,7 +853,9 @@ static void sunxi_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
        case MMC_POWER_OFF:
                dev_dbg(mmc_dev(mmc), "power off!\n");
                sunxi_mmc_reset_host(host);
-               mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0);
+               if (!IS_ERR(mmc->supply.vmmc))
+                       mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0);
+
                if (!IS_ERR(mmc->supply.vqmmc) && host->vqmmc_enabled)
                        regulator_disable(mmc->supply.vqmmc);
                host->vqmmc_enabled = false;
--
2.9.3
Otherwise this looks good to me!

Kind regards
Uffe
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help