[PATCH] regulator: rt4801: Fix NULL pointer dereference if priv->enable_gpios is NULL

[PATCH] regulator: rt4801: Fix NULL pointer dereference if priv->enable_gpios is NULL

[Axel Lin]

devm_gpiod_get_array_optional may return NULL if no GPIO was assigned.

Signed-off-by: Axel Lin <axel.lin@ingics.com>
---
 drivers/regulator/rt4801-regulator.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/rt4801-regulator.c b/drivers/regulator/rt4801-regulator.c
index 2055a9cb13ba..7a87788d3f09 100644
--- a/drivers/regulator/rt4801-regulator.c
+++ b/drivers/regulator/rt4801-regulator.c
@@ -66,7 +66,7 @@ static int rt4801_enable(struct regulator_dev *rdev)
 	struct gpio_descs *gpios = priv->enable_gpios;
 	int id = rdev_get_id(rdev), ret;
 
-	if (gpios->ndescs <= id) {
+	if (!gpios || gpios->ndescs <= id) {
 		dev_warn(&rdev->dev, "no dedicated gpio can control\n");
 		goto bypass_gpio;
 	}
@@ -88,7 +88,7 @@ static int rt4801_disable(struct regulator_dev *rdev)
 	struct gpio_descs *gpios = priv->enable_gpios;
 	int id = rdev_get_id(rdev);
 
-	if (gpios->ndescs <= id) {
+	if (!gpios || gpios->ndescs <= id) {
 		dev_warn(&rdev->dev, "no dedicated gpio can control\n");
 		goto bypass_gpio;
 	}
-- 
2.25.1

Re: [PATCH] regulator: rt4801: Fix NULL pointer dereference if priv->enable_gpios is NULL

[cy_huang(黃啟原)]

Hi, Axel:

Which case will cause this error? I'm not really sure.
But if devm_gpiod_get_array_optional will return null, then it must be check
earlier in probe function

        priv->enable_gpios = devm_gpiod_get_array_optional(&i2c->dev, "enable",
GPIOD_OUT_HIGH);
-       if (IS_ERR(priv->enable_gpios)) {+       if (IS_ERR_OR_NULL(priv->enable_gpios)) {

If so, this change will be more reasonable.
Cause in binding document, I already write the min item must be '1'.
************* Email Confidentiality Notice ********************

The information contained in this e-mail message (including any attachments) may be confidential, proprietary, privileged, or otherwise exempt from disclosure under applicable laws. It is intended to be conveyed only to the designated recipient(s). Any use, dissemination, distribution, printing, retaining or copying of this e-mail (including its attachments) by unintended recipient(s) is strictly prohibited and may be unlawful. If you are not an intended recipient of this e-mail, or believe that you have received this e-mail in error, please notify the sender immediately (by replying to this e-mail), delete any and all copies of this e-mail (including any attachments) from your system, and do not disclose the content of this e-mail to any other person. Thank you!

Re: [PATCH] regulator: rt4801: Fix NULL pointer dereference if priv->enable_gpios is NULL

[Axel Lin]

cy_huang(黃啟原) <cy_huang@richtek.com> 於 2021年6月3日 週四 下午6:39寫道：
>
> Hi, Axel:
>
> Which case will cause this error? I'm not really sure.
> But if devm_gpiod_get_array_optional will return null, then it must be check
> earlier in probe function
>
>         priv->enable_gpios = devm_gpiod_get_array_optional(&i2c->dev, "enable",
> GPIOD_OUT_HIGH);
> -       if (IS_ERR(priv->enable_gpios)) {+       if (IS_ERR_OR_NULL(priv->enable_gpios)) {
>
> If so, this change will be more reasonable.
> Cause in binding document, I already write the min item must be '1'.

Documentation/devicetree/bindings/regulator/richtek,rt4801-regulator.yaml
  enable-gpios:
    description: GPIOs to use to enable DSVP/DSVN regulator.
      The first one is ENP to enable DSVP, and second one is ENM to enable DSVN.
      Number of GPIO in the array list could be 1 or 2.
      If only one gpio is specified, only one gpio used to control ENP/ENM.
      Else both are spefied, DSVP/DSVN could be controlled individually.
      Othersie, this property not specified. treat both as always-on regulator.
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this  imply it's optional.

If it's cannot be optional, you should use devm_gpiod_get_array() instead.

Regards,
Axel

Re: [PATCH] regulator: rt4801: Fix NULL pointer dereference if priv->enable_gpios is NULL

[cy_huang(黃啟原)]

Hi,Sorry, I miserunderstand it.
It's really optional as you said.
I really don't think about the case you specified.
Thanks for the fix.
>
> Regards,
> Axel
************* Email Confidentiality Notice ********************

The information contained in this e-mail message (including any attachments) may be confidential, proprietary, privileged, or otherwise exempt from disclosure under applicable laws. It is intended to be conveyed only to the designated recipient(s). Any use, dissemination, distribution, printing, retaining or copying of this e-mail (including its attachments) by unintended recipient(s) is strictly prohibited and may be unlawful. If you are not an intended recipient of this e-mail, or believe that you have received this e-mail in error, please notify the sender immediately (by replying to this e-mail), delete any and all copies of this e-mail (including any attachments) from your system, and do not disclose the content of this e-mail to any other person. Thank you!

Re: [PATCH] regulator: rt4801: Fix NULL pointer dereference if priv->enable_gpios is NULL

[Mark Brown]

On Thu, 3 Jun 2021 17:49:44 +0800, Axel Lin wrote:
> devm_gpiod_get_array_optional may return NULL if no GPIO was assigned.

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next

Thanks!

[1/1] regulator: rt4801: Fix NULL pointer dereference if priv->enable_gpios is NULL
      commit: cb2381cbecb81a8893b2d1e1af29bc2e5531df27

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark