Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1503625 > unrolled thread

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

Started byMaxime Ripard <maxime.ripard@free-electrons.com>
First post2016-10-19 16:20 +0200
Last post2016-10-21 10:30 +0200
Articles 3 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v2] mmc: sunxi: Prevent against null dereference for vmmc Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-10-19 16:20 +0200
    Re: [PATCH v2] mmc: sunxi: Prevent against null dereference for vmmc Chen-Yu Tsai <wens@csie.org> - 2016-10-21 05:00 +0200
    Re: [PATCH v2] mmc: sunxi: Prevent against null dereference for vmmc Ulf Hansson <ulf.hansson@linaro.org> - 2016-10-21 10:30 +0200

#1503625 — [PATCH v2] mmc: sunxi: Prevent against null dereference for vmmc

FromMaxime Ripard <maxime.ripard@free-electrons.com>
Date2016-10-19 16:20 +0200
Subject[PATCH v2] mmc: sunxi: Prevent against null dereference for vmmc
Message-ID<stZSP-2nd-105@gated-at.bofh.it>
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 <maxime.ripard@free-electrons.com>

---
Changes from v1:
  - remove redundant error message
---
 drivers/mmc/host/sunxi-mmc.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index c0a5c676d0e8..b1d1303389a7 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -822,10 +822,13 @@ 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)
+				return;
+		}
 
 		if (!IS_ERR(mmc->supply.vqmmc)) {
 			host->ferror = regulator_enable(mmc->supply.vqmmc);
@@ -847,7 +850,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

[toc] | [next] | [standalone]


#1505424

FromChen-Yu Tsai <wens@csie.org>
Date2016-10-21 05:00 +0200
Message-ID<suydP-7Zt-7@gated-at.bofh.it>
In reply to#1503625
On Wed, Oct 19, 2016 at 9:33 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> 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 <maxime.ripard@free-electrons.com>

Acked-by: Chen-Yu Tsai <wens@csie.org>

[toc] | [prev] | [next] | [standalone]


#1505561

FromUlf Hansson <ulf.hansson@linaro.org>
Date2016-10-21 10:30 +0200
Message-ID<suDnb-30q-1@gated-at.bofh.it>
In reply to#1503625
On 19 October 2016 at 15:33, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> 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 <maxime.ripard@free-electrons.com>

Thanks, applied for next!

Kind regards
Uffe

>
> ---
> Changes from v1:
>   - remove redundant error message
> ---
>  drivers/mmc/host/sunxi-mmc.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
> index c0a5c676d0e8..b1d1303389a7 100644
> --- a/drivers/mmc/host/sunxi-mmc.c
> +++ b/drivers/mmc/host/sunxi-mmc.c
> @@ -822,10 +822,13 @@ 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)
> +                               return;
> +               }
>
>                 if (!IS_ERR(mmc->supply.vqmmc)) {
>                         host->ferror = regulator_enable(mmc->supply.vqmmc);
> @@ -847,7 +850,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
>

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web