[PATCH] regulator: s2mps11: Fix ERR_PTR dereference on GPIO lookup failure

[PATCH] regulator: s2mps11: Fix ERR_PTR dereference on GPIO lookup failure

[Krzysztof Kozlowski]

If devm_gpiod_get_from_of_node() call returns ERR_PTR, it is assigned
into an array of GPIO descriptors and used later because such error is
not treated as critical thus it is not propagated back to the probe
function.

All code later expects that such GPIO descriptor is either a NULL or
proper value.  This later might lead to dereference of ERR_PTR.

Only devices with S2MPS14 flavor are affected (other do not control
regulators with GPIOs).

Fixes: 1c984942f0a4 ("regulator: s2mps11: Pass descriptor instead of GPIO number")
Cc: <stable@vger.kernel.org>
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>

---

The exact error condition was not tested because I do not have board
with S2MPS14.
---
 drivers/regulator/s2mps11.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c
index 134c62db36c5..af9bf10b4c33 100644
--- a/drivers/regulator/s2mps11.c
+++ b/drivers/regulator/s2mps11.c
@@ -824,6 +824,7 @@ static void s2mps14_pmic_dt_parse_ext_control_gpio(struct platform_device *pdev,
 		if (IS_ERR(gpio[reg])) {
 			dev_err(&pdev->dev, "Failed to get control GPIO for %d/%s\n",
 				reg, rdata[reg].name);
+			gpio[reg] = NULL;
 			continue;
 		}
 		if (gpio[reg])
-- 
2.7.4

Applied "regulator: s2mps11: Fix ERR_PTR dereference on GPIO lookup failure" to the regulator tree

[Mark Brown]

The patch

   regulator: s2mps11: Fix ERR_PTR dereference on GPIO lookup failure

has been applied to the regulator tree at

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

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

>From 70ca117b02f3b1c8830fe95e4e3dea2937038e11 Mon Sep 17 00:00:00 2001
From: Krzysztof Kozlowski <krzk@kernel.org>
Date: Wed, 19 Jun 2019 14:42:39 +0200
Subject: [PATCH] regulator: s2mps11: Fix ERR_PTR dereference on GPIO lookup
 failure

If devm_gpiod_get_from_of_node() call returns ERR_PTR, it is assigned
into an array of GPIO descriptors and used later because such error is
not treated as critical thus it is not propagated back to the probe
function.

All code later expects that such GPIO descriptor is either a NULL or
proper value.  This later might lead to dereference of ERR_PTR.

Only devices with S2MPS14 flavor are affected (other do not control
regulators with GPIOs).

Fixes: 1c984942f0a4 ("regulator: s2mps11: Pass descriptor instead of GPIO number")
Cc: <stable@vger.kernel.org>
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/regulator/s2mps11.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c
index 134c62db36c5..af9bf10b4c33 100644
--- a/drivers/regulator/s2mps11.c
+++ b/drivers/regulator/s2mps11.c
@@ -824,6 +824,7 @@ static void s2mps14_pmic_dt_parse_ext_control_gpio(struct platform_device *pdev,
 		if (IS_ERR(gpio[reg])) {
 			dev_err(&pdev->dev, "Failed to get control GPIO for %d/%s\n",
 				reg, rdata[reg].name);
+			gpio[reg] = NULL;
 			continue;
 		}
 		if (gpio[reg])
-- 
2.20.1

Applied "regulator: s2mps11: Fix ERR_PTR dereference on GPIO lookup failure" to the regulator tree

[Mark Brown]

The patch

   regulator: s2mps11: Fix ERR_PTR dereference on GPIO lookup failure

has been applied to the regulator tree at

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

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

From 70ca117b02f3b1c8830fe95e4e3dea2937038e11 Mon Sep 17 00:00:00 2001
From: Krzysztof Kozlowski <krzk@kernel.org>
Date: Wed, 19 Jun 2019 14:42:39 +0200
Subject: [PATCH] regulator: s2mps11: Fix ERR_PTR dereference on GPIO lookup
 failure

If devm_gpiod_get_from_of_node() call returns ERR_PTR, it is assigned
into an array of GPIO descriptors and used later because such error is
not treated as critical thus it is not propagated back to the probe
function.

All code later expects that such GPIO descriptor is either a NULL or
proper value.  This later might lead to dereference of ERR_PTR.

Only devices with S2MPS14 flavor are affected (other do not control
regulators with GPIOs).

Fixes: 1c984942f0a4 ("regulator: s2mps11: Pass descriptor instead of GPIO number")
Cc: <stable@vger.kernel.org>
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/regulator/s2mps11.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c
index 134c62db36c5..af9bf10b4c33 100644
--- a/drivers/regulator/s2mps11.c
+++ b/drivers/regulator/s2mps11.c
@@ -824,6 +824,7 @@ static void s2mps14_pmic_dt_parse_ext_control_gpio(struct platform_device *pdev,
 		if (IS_ERR(gpio[reg])) {
 			dev_err(&pdev->dev, "Failed to get control GPIO for %d/%s\n",
 				reg, rdata[reg].name);
+			gpio[reg] = NULL;
 			continue;
 		}
 		if (gpio[reg])
-- 
2.20.1

Re: [PATCH] regulator: s2mps11: Fix ERR_PTR dereference on GPIO lookup failure

[Krzysztof Kozlowski]

On Sat, 22 Jun 2019 at 20:13, Sasha Levin <sashal@kernel.org> wrote:
>
> Hi,
>
> [This is an automated email]
>
> This commit has been processed because it contains a "Fixes:" tag,
> fixing commit: 1c984942f0a4 regulator: s2mps11: Pass descriptor instead of GPIO number.
>
> The bot has tested the following trees: v5.1.12, v4.19.53.
>
> v5.1.12: Build OK!
> v4.19.53: Failed to apply! Possible dependencies:
>     Unable to calculate
>
>
> How should we proceed with this patch?

The commit mentioned in fixes tag (1c984942f0a4) came in
v5.0-rc1/v5.0. Why do you try to port it to v4.19?

Best regards,
Krzysztof

Re: [PATCH] regulator: s2mps11: Fix ERR_PTR dereference on GPIO lookup failure

[Sasha Levin]

On Mon, Jun 24, 2019 at 09:01:27AM +0200, Krzysztof Kozlowski wrote:
>On Sat, 22 Jun 2019 at 20:13, Sasha Levin <sashal@kernel.org> wrote:
>>
>> Hi,
>>
>> [This is an automated email]
>>
>> This commit has been processed because it contains a "Fixes:" tag,
>> fixing commit: 1c984942f0a4 regulator: s2mps11: Pass descriptor instead of GPIO number.
>>
>> The bot has tested the following trees: v5.1.12, v4.19.53.
>>
>> v5.1.12: Build OK!
>> v4.19.53: Failed to apply! Possible dependencies:
>>     Unable to calculate
>>
>>
>> How should we proceed with this patch?
>
>The commit mentioned in fixes tag (1c984942f0a4) came in
>v5.0-rc1/v5.0. Why do you try to port it to v4.19?

This is an interesting one! Usually when something like this happens, it
means that the "fixed" commit was backported to stable, but as you
pointed out, it was not.

My scripts have logic to try and detect these backports, and it appears
that there's a commit with an identical subject line in the 4.19 tree,
so at this point there are two commits with identical subject lines in
Linus's tree:

1c984942f0a4 regulator: s2mps11: Pass descriptor instead of GPIO number
0369e02b75e6 regulator: s2mps11: Pass descriptor instead of GPIO number

Which can be confusing for humans too :)

--
Thanks,
Sasha

Re: [PATCH] regulator: s2mps11: Fix ERR_PTR dereference on GPIO lookup failure

[Krzysztof Kozlowski]

On Mon, 24 Jun 2019 at 12:11, Sasha Levin <sashal@kernel.org> wrote:
>
> On Mon, Jun 24, 2019 at 09:01:27AM +0200, Krzysztof Kozlowski wrote:
> >On Sat, 22 Jun 2019 at 20:13, Sasha Levin <sashal@kernel.org> wrote:
> >>
> >> Hi,
> >>
> >> [This is an automated email]
> >>
> >> This commit has been processed because it contains a "Fixes:" tag,
> >> fixing commit: 1c984942f0a4 regulator: s2mps11: Pass descriptor instead of GPIO number.
> >>
> >> The bot has tested the following trees: v5.1.12, v4.19.53.
> >>
> >> v5.1.12: Build OK!
> >> v4.19.53: Failed to apply! Possible dependencies:
> >>     Unable to calculate
> >>
> >>
> >> How should we proceed with this patch?
> >
> >The commit mentioned in fixes tag (1c984942f0a4) came in
> >v5.0-rc1/v5.0. Why do you try to port it to v4.19?
>
> This is an interesting one! Usually when something like this happens, it
> means that the "fixed" commit was backported to stable, but as you
> pointed out, it was not.
>
> My scripts have logic to try and detect these backports, and it appears
> that there's a commit with an identical subject line in the 4.19 tree,
> so at this point there are two commits with identical subject lines in
> Linus's tree:
>
> 1c984942f0a4 regulator: s2mps11: Pass descriptor instead of GPIO number
> 0369e02b75e6 regulator: s2mps11: Pass descriptor instead of GPIO number
>
> Which can be confusing for humans too :)

+CC Mark Brown,

Ugh, I see more of commits from this patchset getting to mainline
twice.  One branch goes to Linus via
79f20778fb228ae372cd7602745382fd4543ef31 Merge tag 'regulator-v4.21'
of git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator
which is based on v4.20-rc1

The other commits gets there via:
68cc38ff33f38424d0456f9a1ecfec4683226a7e Merge tag 'regulator-v4.18'
of git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator
which is before (gets into v4.18-rc1).

But how the latter can be applied again if it is based on something
already containing these commits... I am so confused...

Anyway the Fixes needs to be corrected to contain both (also
0369e02b75e6) and should be backported to v4.19... I'll see if I have
spare time to make a backport.

Best regards,
Krzysztof

Re: [PATCH] regulator: s2mps11: Fix ERR_PTR dereference on GPIO lookup failure

[Krzysztof Kozlowski]

 On Mon, 24 Jun 2019 at 12:41, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Mon, 24 Jun 2019 at 12:11, Sasha Levin <sashal@kernel.org> wrote:
> >
> > On Mon, Jun 24, 2019 at 09:01:27AM +0200, Krzysztof Kozlowski wrote:
> > >On Sat, 22 Jun 2019 at 20:13, Sasha Levin <sashal@kernel.org> wrote:
> > >>
> > >> Hi,
> > >>
> > >> [This is an automated email]
> > >>
> > >> This commit has been processed because it contains a "Fixes:" tag,
> > >> fixing commit: 1c984942f0a4 regulator: s2mps11: Pass descriptor instead of GPIO number.
> > >>
> > >> The bot has tested the following trees: v5.1.12, v4.19.53.
> > >>
> > >> v5.1.12: Build OK!
> > >> v4.19.53: Failed to apply! Possible dependencies:
> > >>     Unable to calculate
> > >>
> > >>
> > >> How should we proceed with this patch?
> > >
> > >The commit mentioned in fixes tag (1c984942f0a4) came in
> > >v5.0-rc1/v5.0. Why do you try to port it to v4.19?
> >
> > This is an interesting one! Usually when something like this happens, it
> > means that the "fixed" commit was backported to stable, but as you
> > pointed out, it was not.
> >
> > My scripts have logic to try and detect these backports, and it appears
> > that there's a commit with an identical subject line in the 4.19 tree,
> > so at this point there are two commits with identical subject lines in
> > Linus's tree:
> >
> > 1c984942f0a4 regulator: s2mps11: Pass descriptor instead of GPIO number
> > 0369e02b75e6 regulator: s2mps11: Pass descriptor instead of GPIO number
> >
> > Which can be confusing for humans too :)
>
> +CC Mark Brown,
>
> Ugh, I see more of commits from this patchset getting to mainline
> twice.  One branch goes to Linus via
> 79f20778fb228ae372cd7602745382fd4543ef31 Merge tag 'regulator-v4.21'
> of git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator
> which is based on v4.20-rc1
>
> The other commits gets there via:
> 68cc38ff33f38424d0456f9a1ecfec4683226a7e Merge tag 'regulator-v4.18'
> of git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator
> which is before (gets into v4.18-rc1).
>
> But how the latter can be applied again if it is based on something
> already containing these commits... I am so confused...
>
> Anyway the Fixes needs to be corrected to contain both (also
> 0369e02b75e6) and should be backported to v4.19... I'll see if I have
> spare time to make a backport.

Ah, I see now revert of the first commit
37fa23dbccbd97663acc085bd79246f427e603a1 (regulator: s2mps11: Fix boot
on Odroid XU3). Actually git diff points that there was no effective
change in the s2mps11 regulator.

Since the revert got into v4.18, then v4.19 is safe and there is no
need for backport.

Best regards,
Krzysztof