linux-omap.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Nishanth Menon <nm@ti.com>
To: Mark Brown <broonie@kernel.org>, Liam Girdwood <lgirdwood@gmail.com>
Cc: <linux-kernel@vger.kernel.org>, <linux-omap@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<lkft-triage@lists.linaro.org>, <linux-pm@vger.kernel.org>,
	Naresh Kamboju <naresh.kamboju@linaro.org>,
	Arnd Bergmann <arnd@arndb.de>, Nishanth Menon <nm@ti.com>
Subject: [PATCH] regulator: ti-abb: Fix array out of bound read access on the first transition
Date: Wed, 18 Nov 2020 08:50:09 -0600	[thread overview]
Message-ID: <20201118145009.10492-1-nm@ti.com> (raw)

At the start of driver initialization, we do not know what bias
setting the bootloader has configured the system for and we only know
for certain the very first time we do a transition.

However, since the initial value of the comparison index is -EINVAL,
this negative value results in an array out of bound access on the
very first transition.

Since we don't know what the setting is, we just set the bias
configuration as there is nothing to compare against. This prevents
the array out of bound access.

NOTE: Even though we could use a more relaxed check of "< 0" the only
valid values(ignoring cosmic ray induced bitflips) are -EINVAL, 0+.

Fixes: 40b1936efebd ("regulator: Introduce TI Adaptive Body Bias(ABB) on-chip LDO driver")
Link: https://lore.kernel.org/linux-mm/CA+G9fYuk4imvhyCN7D7T6PMDH6oNp6HDCRiTUKMQ6QXXjBa4ag@mail.gmail.com/
Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Nishanth Menon <nm@ti.com>
---

Mark,

I will leave it to your descretion if this needs to be tagged for
stable or to drop the Fixes tag - Side effect, if this occurs, will be
an unstable system very hard to track down - but typically occurring
during system boot - Impacts systems: DM3/OMAP3,4,5,DRA7/AM5x.

I would categorize this as "This could be a problem..." problem..
the bug is an out of bound read, and has been around since v3.11 and is
not a catastrophic data corruption kind of issue.

Though theoretically, there is a possibility that the compare may
pass and result in missing bias configuration(and resulting system
will be unstable), I have'nt heard of actual report (but, it will be
surprising if any actual instability was actually tracked down to this
bug). Any ways, I had to go to git full history to pick the exact
commit - I have left it in the patch.


Arnd,

I have left your reviewed-by from the thread, I can fixup
any further commit message comments if you may have.

drivers/regulator/ti-abb-regulator.c | 12 +++++++++++- 1 file changed,
11 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/ti-abb-regulator.c b/drivers/regulator/ti-abb-regulator.c
index 3e60bff76194..9f0a4d50cead 100644
--- a/drivers/regulator/ti-abb-regulator.c
+++ b/drivers/regulator/ti-abb-regulator.c
@@ -342,8 +342,17 @@ static int ti_abb_set_voltage_sel(struct regulator_dev *rdev, unsigned sel)
 		return ret;
 	}
 
-	/* If data is exactly the same, then just update index, no change */
 	info = &abb->info[sel];
+	/*
+	 * When Linux kernel is starting up, we are'nt sure of the
+	 * Bias configuration that bootloader has configured.
+	 * So, we get to know the actual setting the first time
+	 * we are asked to transition.
+	 */
+	if (abb->current_info_idx == -EINVAL)
+		goto just_set_abb;
+
+	/* If data is exactly the same, then just update index, no change */
 	oinfo = &abb->info[abb->current_info_idx];
 	if (!memcmp(info, oinfo, sizeof(*info))) {
 		dev_dbg(dev, "%s: Same data new idx=%d, old idx=%d\n", __func__,
@@ -351,6 +360,7 @@ static int ti_abb_set_voltage_sel(struct regulator_dev *rdev, unsigned sel)
 		goto out;
 	}
 
+just_set_abb:
 	ret = ti_abb_set_opp(rdev, abb, info);
 
 out:
-- 
2.29.2


             reply	other threads:[~2020-11-18 14:51 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-18 14:50 Nishanth Menon [this message]
2020-11-18 15:38 ` [PATCH] regulator: ti-abb: Fix array out of bound read access on the first transition Andreas Kemnade
2020-11-18 16:24   ` Nishanth Menon
2020-11-18 20:59 ` Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201118145009.10492-1-nm@ti.com \
    --to=nm@ti.com \
    --cc=arnd@arndb.de \
    --cc=broonie@kernel.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=lkft-triage@lists.linaro.org \
    --cc=naresh.kamboju@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).