* [PATCH][V2][next] regulator: axp20x: check rdev is null before dereferencing it
@ 2018-12-22 11:31 Colin King
2019-02-04 19:13 ` Colin Ian King
2019-02-05 3:37 ` Chen-Yu Tsai
0 siblings, 2 replies; 6+ messages in thread
From: Colin King @ 2018-12-22 11:31 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Chen-Yu Tsai; +Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Currently rdev is dereferenced when assigning desc before rdev is null
checked, hence there is a potential null pointer dereference on rdev.
Fix this by null checking rdev first.
Detected by CoverityScan, CID#1476031 ("Dereference before null check")
Fixes: 77e3e3b165db ("regulator: axp20x: add software based soft_start for AXP209 LDO3")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
V2: Fix 2nd occurrance of this thinko
---
drivers/regulator/axp20x-regulator.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
index 48af859fd053..0dfa4ea6bbdf 100644
--- a/drivers/regulator/axp20x-regulator.c
+++ b/drivers/regulator/axp20x-regulator.c
@@ -367,7 +367,7 @@ static const int axp209_dcdc2_ldo3_slew_rates[] = {
static int axp20x_set_ramp_delay(struct regulator_dev *rdev, int ramp)
{
struct axp20x_dev *axp20x = rdev_get_drvdata(rdev);
- const struct regulator_desc *desc = rdev->desc;
+ const struct regulator_desc *desc;
u8 reg, mask, enable, cfg = 0xff;
const int *slew_rates;
int rate_count = 0;
@@ -375,6 +375,8 @@ static int axp20x_set_ramp_delay(struct regulator_dev *rdev, int ramp)
if (!rdev)
return -EINVAL;
+ desc = rdev->desc;
+
switch (axp20x->variant) {
case AXP209_ID:
if (desc->id == AXP20X_DCDC2) {
@@ -436,11 +438,13 @@ static int axp20x_set_ramp_delay(struct regulator_dev *rdev, int ramp)
static int axp20x_regulator_enable_regmap(struct regulator_dev *rdev)
{
struct axp20x_dev *axp20x = rdev_get_drvdata(rdev);
- const struct regulator_desc *desc = rdev->desc;
+ const struct regulator_desc *desc;
if (!rdev)
return -EINVAL;
+ desc = rdev->desc;
+
switch (axp20x->variant) {
case AXP209_ID:
if ((desc->id == AXP20X_LDO3) &&
--
2.19.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH][V2][next] regulator: axp20x: check rdev is null before dereferencing it
2018-12-22 11:31 [PATCH][V2][next] regulator: axp20x: check rdev is null before dereferencing it Colin King
@ 2019-02-04 19:13 ` Colin Ian King
2019-02-04 19:24 ` Mark Brown
2019-02-04 19:30 ` Dan Carpenter
2019-02-05 3:37 ` Chen-Yu Tsai
1 sibling, 2 replies; 6+ messages in thread
From: Colin Ian King @ 2019-02-04 19:13 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Chen-Yu Tsai; +Cc: kernel-janitors, linux-kernel
ping?
On 22/12/2018 11:31, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Currently rdev is dereferenced when assigning desc before rdev is null
> checked, hence there is a potential null pointer dereference on rdev.
> Fix this by null checking rdev first.
>
> Detected by CoverityScan, CID#1476031 ("Dereference before null check")
>
> Fixes: 77e3e3b165db ("regulator: axp20x: add software based soft_start for AXP209 LDO3")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>
> V2: Fix 2nd occurrance of this thinko
>
> ---
> drivers/regulator/axp20x-regulator.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
> index 48af859fd053..0dfa4ea6bbdf 100644
> --- a/drivers/regulator/axp20x-regulator.c
> +++ b/drivers/regulator/axp20x-regulator.c
> @@ -367,7 +367,7 @@ static const int axp209_dcdc2_ldo3_slew_rates[] = {
> static int axp20x_set_ramp_delay(struct regulator_dev *rdev, int ramp)
> {
> struct axp20x_dev *axp20x = rdev_get_drvdata(rdev);
> - const struct regulator_desc *desc = rdev->desc;
> + const struct regulator_desc *desc;
> u8 reg, mask, enable, cfg = 0xff;
> const int *slew_rates;
> int rate_count = 0;
> @@ -375,6 +375,8 @@ static int axp20x_set_ramp_delay(struct regulator_dev *rdev, int ramp)
> if (!rdev)
> return -EINVAL;
>
> + desc = rdev->desc;
> +
> switch (axp20x->variant) {
> case AXP209_ID:
> if (desc->id == AXP20X_DCDC2) {
> @@ -436,11 +438,13 @@ static int axp20x_set_ramp_delay(struct regulator_dev *rdev, int ramp)
> static int axp20x_regulator_enable_regmap(struct regulator_dev *rdev)
> {
> struct axp20x_dev *axp20x = rdev_get_drvdata(rdev);
> - const struct regulator_desc *desc = rdev->desc;
> + const struct regulator_desc *desc;
>
> if (!rdev)
> return -EINVAL;
>
> + desc = rdev->desc;
> +
> switch (axp20x->variant) {
> case AXP209_ID:
> if ((desc->id == AXP20X_LDO3) &&
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH][V2][next] regulator: axp20x: check rdev is null before dereferencing it
2019-02-04 19:13 ` Colin Ian King
@ 2019-02-04 19:24 ` Mark Brown
2019-02-04 19:30 ` Dan Carpenter
1 sibling, 0 replies; 6+ messages in thread
From: Mark Brown @ 2019-02-04 19:24 UTC (permalink / raw)
To: Colin Ian King; +Cc: Liam Girdwood, Chen-Yu Tsai, kernel-janitors, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 839 bytes --]
On Mon, Feb 04, 2019 at 07:13:30PM +0000, Colin Ian King wrote:
> ping?
Please don't send content free pings and please allow a reasonable time
for review. People get busy, go on holiday, attend conferences and so
on so unless there is some reason for urgency (like critical bug fixes)
please allow at least a couple of weeks for review. If there have been
review comments then people may be waiting for those to be addressed.
Sending content free pings adds to the mail volume (if they are seen at
all) which is often the problem and since they can't be reviewed
directly if something has gone wrong you'll have to resend the patches
anyway, so sending again is generally a better approach though there are
some other maintainers who like them - if in doubt look at how patches
for the subsystem are normally handled.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][V2][next] regulator: axp20x: check rdev is null before dereferencing it
2019-02-04 19:13 ` Colin Ian King
2019-02-04 19:24 ` Mark Brown
@ 2019-02-04 19:30 ` Dan Carpenter
1 sibling, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2019-02-04 19:30 UTC (permalink / raw)
To: Colin Ian King
Cc: Liam Girdwood, Mark Brown, Chen-Yu Tsai, kernel-janitors,
linux-kernel
On Mon, Feb 04, 2019 at 07:13:30PM +0000, Colin Ian King wrote:
> > drivers/regulator/axp20x-regulator.c | 8 ++++++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
> > index 48af859fd053..0dfa4ea6bbdf 100644
> > --- a/drivers/regulator/axp20x-regulator.c
> > +++ b/drivers/regulator/axp20x-regulator.c
> > @@ -367,7 +367,7 @@ static const int axp209_dcdc2_ldo3_slew_rates[] = {
> > static int axp20x_set_ramp_delay(struct regulator_dev *rdev, int ramp)
> > {
> > struct axp20x_dev *axp20x = rdev_get_drvdata(rdev);
> > - const struct regulator_desc *desc = rdev->desc;
> > + const struct regulator_desc *desc;
This is only called from set_machine_constraints() so "rdev" can't be
NULL. (On my allmodonconfig build).
> > u8 reg, mask, enable, cfg = 0xff;
> > const int *slew_rates;
> > int rate_count = 0;
> > @@ -375,6 +375,8 @@ static int axp20x_set_ramp_delay(struct regulator_dev *rdev, int ramp)
> > if (!rdev)
> > return -EINVAL;
> >
> > + desc = rdev->desc;
> > +
> > switch (axp20x->variant) {
> > case AXP209_ID:
> > if (desc->id == AXP20X_DCDC2) {
> > @@ -436,11 +438,13 @@ static int axp20x_set_ramp_delay(struct regulator_dev *rdev, int ramp)
> > static int axp20x_regulator_enable_regmap(struct regulator_dev *rdev)
> > {
> > struct axp20x_dev *axp20x = rdev_get_drvdata(rdev);
> > - const struct regulator_desc *desc = rdev->desc;
> > + const struct regulator_desc *desc;
> >
> > if (!rdev)
> > return -EINVAL;
> >
> > + desc = rdev->desc;
Called from _regulator_do_enable() and rdev can't be NULL:
ret = rdev->desc->ops->enable(rdev);
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][V2][next] regulator: axp20x: check rdev is null before dereferencing it
2018-12-22 11:31 [PATCH][V2][next] regulator: axp20x: check rdev is null before dereferencing it Colin King
2019-02-04 19:13 ` Colin Ian King
@ 2019-02-05 3:37 ` Chen-Yu Tsai
2019-02-15 7:46 ` Dan Carpenter
1 sibling, 1 reply; 6+ messages in thread
From: Chen-Yu Tsai @ 2019-02-05 3:37 UTC (permalink / raw)
To: Colin King; +Cc: Liam Girdwood, Mark Brown, kernel-janitors, linux-kernel
On Sat, Dec 22, 2018 at 7:32 PM Colin King <colin.king@canonical.com> wrote:
>
> From: Colin Ian King <colin.king@canonical.com>
>
> Currently rdev is dereferenced when assigning desc before rdev is null
> checked, hence there is a potential null pointer dereference on rdev.
> Fix this by null checking rdev first.
>
> Detected by CoverityScan, CID#1476031 ("Dereference before null check")
>
> Fixes: 77e3e3b165db ("regulator: axp20x: add software based soft_start for AXP209 LDO3")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
This patch somehow fell through the cracks. Sorry about that.
Both are really false positives. Both functions are callbacks for the
regulator core to use. If you look through the core code, rdev->desc->ops
is used to check for these callbacks before they are called. rdev is always
passed in, and rdev->desc is always valid after regulator_register.
ChenYu
> ---
>
> V2: Fix 2nd occurrance of this thinko
>
> ---
> drivers/regulator/axp20x-regulator.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
> index 48af859fd053..0dfa4ea6bbdf 100644
> --- a/drivers/regulator/axp20x-regulator.c
> +++ b/drivers/regulator/axp20x-regulator.c
> @@ -367,7 +367,7 @@ static const int axp209_dcdc2_ldo3_slew_rates[] = {
> static int axp20x_set_ramp_delay(struct regulator_dev *rdev, int ramp)
> {
> struct axp20x_dev *axp20x = rdev_get_drvdata(rdev);
> - const struct regulator_desc *desc = rdev->desc;
> + const struct regulator_desc *desc;
> u8 reg, mask, enable, cfg = 0xff;
> const int *slew_rates;
> int rate_count = 0;
> @@ -375,6 +375,8 @@ static int axp20x_set_ramp_delay(struct regulator_dev *rdev, int ramp)
> if (!rdev)
> return -EINVAL;
>
> + desc = rdev->desc;
> +
> switch (axp20x->variant) {
> case AXP209_ID:
> if (desc->id == AXP20X_DCDC2) {
> @@ -436,11 +438,13 @@ static int axp20x_set_ramp_delay(struct regulator_dev *rdev, int ramp)
> static int axp20x_regulator_enable_regmap(struct regulator_dev *rdev)
> {
> struct axp20x_dev *axp20x = rdev_get_drvdata(rdev);
> - const struct regulator_desc *desc = rdev->desc;
> + const struct regulator_desc *desc;
>
> if (!rdev)
> return -EINVAL;
>
> + desc = rdev->desc;
> +
> switch (axp20x->variant) {
> case AXP209_ID:
> if ((desc->id == AXP20X_LDO3) &&
> --
> 2.19.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH][V2][next] regulator: axp20x: check rdev is null before dereferencing it
2019-02-05 3:37 ` Chen-Yu Tsai
@ 2019-02-15 7:46 ` Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2019-02-15 7:46 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Colin King, Liam Girdwood, Mark Brown, kernel-janitors,
linux-kernel
On Tue, Feb 05, 2019 at 11:37:40AM +0800, Chen-Yu Tsai wrote:
> On Sat, Dec 22, 2018 at 7:32 PM Colin King <colin.king@canonical.com> wrote:
> >
> > From: Colin Ian King <colin.king@canonical.com>
> >
> > Currently rdev is dereferenced when assigning desc before rdev is null
> > checked, hence there is a potential null pointer dereference on rdev.
> > Fix this by null checking rdev first.
> >
> > Detected by CoverityScan, CID#1476031 ("Dereference before null check")
> >
> > Fixes: 77e3e3b165db ("regulator: axp20x: add software based soft_start for AXP209 LDO3")
> > Signed-off-by: Colin Ian King <colin.king@canonical.com>
>
> This patch somehow fell through the cracks. Sorry about that.
>
> Both are really false positives.
I don't like when people use the word "false positive" like this. The
code doesn't checks for NULL after a dereference so that's a nonsense
to do. The correct thing is to remove the NULL check.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-02-15 7:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-12-22 11:31 [PATCH][V2][next] regulator: axp20x: check rdev is null before dereferencing it Colin King
2019-02-04 19:13 ` Colin Ian King
2019-02-04 19:24 ` Mark Brown
2019-02-04 19:30 ` Dan Carpenter
2019-02-05 3:37 ` Chen-Yu Tsai
2019-02-15 7:46 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox