2019-03-09 05:19:07

by Kangjie Lu

Subject: [PATCH] iio: hmc: fix a potential NULL pointer dereference

devm_regmap_init_i2c may fail and return NULL. The fix returns
the error when it fails.

Signed-off-by: Kangjie Lu <[email protected]>
---
drivers/iio/magnetometer/hmc5843_i2c.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/magnetometer/hmc5843_i2c.c b/drivers/iio/magnetometer/hmc5843_i2c.c
index 3de7f4426ac4..c0cd0823f8d5 100644
--- a/drivers/iio/magnetometer/hmc5843_i2c.c
+++ b/drivers/iio/magnetometer/hmc5843_i2c.c
@@ -58,8 +58,13 @@ static const struct regmap_config hmc5843_i2c_regmap_config = {
static int hmc5843_i2c_probe(struct i2c_client *cli,
const struct i2c_device_id *id)
{
+ struct regmap *devm_regmap = devm_regmap_init_i2c(cli,
+ &hmc5843_i2c_regmap_config);
+ if (IS_ERR(devm_regmap))
+ return PTR_ERR(devm_regmap);
+
return hmc5843_common_probe(&cli->dev,
- devm_regmap_init_i2c(cli, &hmc5843_i2c_regmap_config),
+ devm_regmap,
id->driver_data, id->name);
}

--
2.17.1

2019-03-09 13:44:11

by Tomasz Duszynski

Subject: Re: [PATCH] iio: hmc: fix a potential NULL pointer dereference

On Fri, Mar 08, 2019 at 11:15:32PM -0600, Kangjie Lu wrote:
> devm_regmap_init_i2c may fail and return NULL. The fix returns
> the error when it fails.
>
> Signed-off-by: Kangjie Lu <[email protected]>
> ---
> drivers/iio/magnetometer/hmc5843_i2c.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/magnetometer/hmc5843_i2c.c b/drivers/iio/magnetometer/hmc5843_i2c.c
> index 3de7f4426ac4..c0cd0823f8d5 100644
> --- a/drivers/iio/magnetometer/hmc5843_i2c.c
> +++ b/drivers/iio/magnetometer/hmc5843_i2c.c
> @@ -58,8 +58,13 @@ static const struct regmap_config hmc5843_i2c_regmap_config = {
> static int hmc5843_i2c_probe(struct i2c_client *cli,
> const struct i2c_device_id *id)
> {
> + struct regmap *devm_regmap = devm_regmap_init_i2c(cli,
> + &hmc5843_i2c_regmap_config);
> + if (IS_ERR(devm_regmap))
> + return PTR_ERR(devm_regmap);

This fixes only one part of the problem leaving identical spi issue
behind. I guess this check should be part of *common_probe().

> +
> return hmc5843_common_probe(&cli->dev,
> - devm_regmap_init_i2c(cli, &hmc5843_i2c_regmap_config),
> + devm_regmap,
> id->driver_data, id->name);
> }
>
> --
> 2.17.1
>

2019-03-09 18:30:33

by Jonathan Cameron

Subject: Re: [PATCH] iio: hmc: fix a potential NULL pointer dereference

On Sat, 9 Mar 2019 14:42:09 +0100
Tomasz Duszynski <[email protected]> wrote:

> On Fri, Mar 08, 2019 at 11:15:32PM -0600, Kangjie Lu wrote:
> > devm_regmap_init_i2c may fail and return NULL. The fix returns
> > the error when it fails.
> >
> > Signed-off-by: Kangjie Lu <[email protected]>
> > ---
> > drivers/iio/magnetometer/hmc5843_i2c.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iio/magnetometer/hmc5843_i2c.c b/drivers/iio/magnetometer/hmc5843_i2c.c
> > index 3de7f4426ac4..c0cd0823f8d5 100644
> > --- a/drivers/iio/magnetometer/hmc5843_i2c.c
> > +++ b/drivers/iio/magnetometer/hmc5843_i2c.c
> > @@ -58,8 +58,13 @@ static const struct regmap_config hmc5843_i2c_regmap_config = {
> > static int hmc5843_i2c_probe(struct i2c_client *cli,
> > const struct i2c_device_id *id)
> > {
> > + struct regmap *devm_regmap = devm_regmap_init_i2c(cli,
> > + &hmc5843_i2c_regmap_config);
> > + if (IS_ERR(devm_regmap))
> > + return PTR_ERR(devm_regmap);
>
> This fixes only one part of the problem leaving identical spi issue
> behind. I guess this check should be part of *common_probe().
It could be, but that somewhat hides the error checking.

I'd prefer to see it fixed like this, but in both here and the spi
file. Both this and spi in one patch would be great!

Thanks,

Jonathan

>
> > +
> > return hmc5843_common_probe(&cli->dev,
> > - devm_regmap_init_i2c(cli, &hmc5843_i2c_regmap_config),
> > + devm_regmap,
> > id->driver_data, id->name);
> > }
> >
> > --
> > 2.17.1
> >

2019-03-16 15:39:01

by Jonathan Cameron

Subject: Re: [PATCH] iio: hmc: fix a potential NULL pointer dereference

On Tue, 12 Mar 2019 02:19:25 -0500
Kangjie Lu <[email protected]> wrote:

> > On Mar 9, 2019, at 12:29 PM, Jonathan Cameron <[email protected]> wrote:
> >
> > On Sat, 9 Mar 2019 14:42:09 +0100
> > Tomasz Duszynski <[email protected] <mailto:[email protected]>> wrote:
> >
> >> On Fri, Mar 08, 2019 at 11:15:32PM -0600, Kangjie Lu wrote:
> >>> devm_regmap_init_i2c may fail and return NULL. The fix returns
> >>> the error when it fails.
> >>>
> >>> Signed-off-by: Kangjie Lu <[email protected]>
> >>> ---
> >>> drivers/iio/magnetometer/hmc5843_i2c.c | 7 ++++++-
> >>> 1 file changed, 6 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/iio/magnetometer/hmc5843_i2c.c b/drivers/iio/magnetometer/hmc5843_i2c.c
> >>> index 3de7f4426ac4..c0cd0823f8d5 100644
> >>> --- a/drivers/iio/magnetometer/hmc5843_i2c.c
> >>> +++ b/drivers/iio/magnetometer/hmc5843_i2c.c
> >>> @@ -58,8 +58,13 @@ static const struct regmap_config hmc5843_i2c_regmap_config = {
> >>> static int hmc5843_i2c_probe(struct i2c_client *cli,
> >>> const struct i2c_device_id *id)
> >>> {
> >>> + struct regmap *devm_regmap = devm_regmap_init_i2c(cli,
> >>> + &hmc5843_i2c_regmap_config);
> >>> + if (IS_ERR(devm_regmap))
> >>> + return PTR_ERR(devm_regmap);
> >>
> >> This fixes only one part of the problem leaving identical spi issue
> >> behind. I guess this check should be part of *common_probe().
> > It could be, but that somewhat hides the error checking.
> >
> > I'd prefer to see it fixed like this, but in both here and the spi
> > file. Both this and spi in one patch would be great!
>
> I’ve submitted the patch for spi as well.
>
Ah. I missed it because of inconsistent patch naming.
Anyhow, there were some minor suggestions for that patch so please
send a v2 with the two combined.

Thanks

Jonathan

> >
> > Thanks,
> >
> > Jonathan
> >
> >>
> >>> +
> >>> return hmc5843_common_probe(&cli->dev,
> >>> - devm_regmap_init_i2c(cli, &hmc5843_i2c_regmap_config),
> >>> + devm_regmap,
> >>> id->driver_data, id->name);
> >>> }
> >>>
> >>> --
> >>> 2.17.1
>

2019-03-16 22:10:56

by Kangjie Lu

Subject: [PATCH v2] iio: hmc5843: fix potential NULL pointer dereferences

devm_regmap_init_i2c may fail and return NULL. The fix returns
the error when it fails.

Signed-off-by: Kangjie Lu <[email protected]>
---
V2: fix the two together
---
drivers/iio/magnetometer/hmc5843_i2c.c | 7 ++++++-
drivers/iio/magnetometer/hmc5843_spi.c | 7 ++++++-
2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/magnetometer/hmc5843_i2c.c b/drivers/iio/magnetometer/hmc5843_i2c.c
index 3de7f4426ac4..86abba5827a2 100644
--- a/drivers/iio/magnetometer/hmc5843_i2c.c
+++ b/drivers/iio/magnetometer/hmc5843_i2c.c
@@ -58,8 +58,13 @@ static const struct regmap_config hmc5843_i2c_regmap_config = {
static int hmc5843_i2c_probe(struct i2c_client *cli,
const struct i2c_device_id *id)
{
+ struct regmap *regmap = devm_regmap_init_i2c(cli,
+ &hmc5843_i2c_regmap_config);
+ if (IS_ERR(regmap))
+ return PTR_ERR(regmap);
+
return hmc5843_common_probe(&cli->dev,
- devm_regmap_init_i2c(cli, &hmc5843_i2c_regmap_config),
+ regmap,
id->driver_data, id->name);
}

diff --git a/drivers/iio/magnetometer/hmc5843_spi.c b/drivers/iio/magnetometer/hmc5843_spi.c
index 535f03a70d63..8355713651d4 100644
--- a/drivers/iio/magnetometer/hmc5843_spi.c
+++ b/drivers/iio/magnetometer/hmc5843_spi.c
@@ -58,6 +58,7 @@ static const struct regmap_config hmc5843_spi_regmap_config = {
static int hmc5843_spi_probe(struct spi_device *spi)
{
int ret;
+ struct regmap *regmap;
const struct spi_device_id *id = spi_get_device_id(spi);

spi->mode = SPI_MODE_3;
@@ -67,8 +68,12 @@ static int hmc5843_spi_probe(struct spi_device *spi)
if (ret)
return ret;

+ regmap = devm_regmap_init(spi, &hmc5843_spi_regmap_config);
+ if (IS_ERR(regmap))
+ return PTR_ERR(devm_regmap);
+
return hmc5843_common_probe(&spi->dev,
- devm_regmap_init_spi(spi, &hmc5843_spi_regmap_config),
+ regmap,
id->driver_data, id->name);
}

--
2.17.1

2019-03-24 10:49:58

by Jonathan Cameron

Subject: Re: [PATCH v2] iio: hmc5843: fix potential NULL pointer dereferences

On Sat, 16 Mar 2019 17:08:33 -0500
Kangjie Lu <[email protected]> wrote:

> devm_regmap_init_i2c may fail and return NULL. The fix returns
> the error when it fails.
>
> Signed-off-by: Kangjie Lu <[email protected]>
Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.
Thanks.

Jonathan

> ---
> V2: fix the two together
> ---
> drivers/iio/magnetometer/hmc5843_i2c.c | 7 ++++++-
> drivers/iio/magnetometer/hmc5843_spi.c | 7 ++++++-
> 2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/magnetometer/hmc5843_i2c.c b/drivers/iio/magnetometer/hmc5843_i2c.c
> index 3de7f4426ac4..86abba5827a2 100644
> --- a/drivers/iio/magnetometer/hmc5843_i2c.c
> +++ b/drivers/iio/magnetometer/hmc5843_i2c.c
> @@ -58,8 +58,13 @@ static const struct regmap_config hmc5843_i2c_regmap_config = {
> static int hmc5843_i2c_probe(struct i2c_client *cli,
> const struct i2c_device_id *id)
> {
> + struct regmap *regmap = devm_regmap_init_i2c(cli,
> + &hmc5843_i2c_regmap_config);
> + if (IS_ERR(regmap))
> + return PTR_ERR(regmap);
> +
> return hmc5843_common_probe(&cli->dev,
> - devm_regmap_init_i2c(cli, &hmc5843_i2c_regmap_config),
> + regmap,
> id->driver_data, id->name);
> }
>
> diff --git a/drivers/iio/magnetometer/hmc5843_spi.c b/drivers/iio/magnetometer/hmc5843_spi.c
> index 535f03a70d63..8355713651d4 100644
> --- a/drivers/iio/magnetometer/hmc5843_spi.c
> +++ b/drivers/iio/magnetometer/hmc5843_spi.c
> @@ -58,6 +58,7 @@ static const struct regmap_config hmc5843_spi_regmap_config = {
> static int hmc5843_spi_probe(struct spi_device *spi)
> {
> int ret;
> + struct regmap *regmap;
> const struct spi_device_id *id = spi_get_device_id(spi);
>
> spi->mode = SPI_MODE_3;
> @@ -67,8 +68,12 @@ static int hmc5843_spi_probe(struct spi_device *spi)
> if (ret)
> return ret;
>
> + regmap = devm_regmap_init(spi, &hmc5843_spi_regmap_config);
> + if (IS_ERR(regmap))
> + return PTR_ERR(devm_regmap);
> +
> return hmc5843_common_probe(&spi->dev,
> - devm_regmap_init_spi(spi, &hmc5843_spi_regmap_config),
> + regmap,
> id->driver_data, id->name);
> }
>

2019-03-24 11:53:11

by Jonathan Cameron

Subject: Re: [PATCH v2] iio: hmc5843: fix potential NULL pointer dereferences

On Sun, 24 Mar 2019 10:48:56 +0000
Jonathan Cameron <[email protected]> wrote:

> On Sat, 16 Mar 2019 17:08:33 -0500
> Kangjie Lu <[email protected]> wrote:
>
> > devm_regmap_init_i2c may fail and return NULL. The fix returns
> > the error when it fails.
> >
> > Signed-off-by: Kangjie Lu <[email protected]>
> Applied to the togreg branch of iio.git and pushed out as testing
> for the autobuilders to play with it.
> Thanks.

Ah, how the eye jumps over the obvious...

Good thing this one was caught by build tests.

See below.

I've fixed up in the tree.

Jonathan

>
> Jonathan
>
> > ---
> > V2: fix the two together
> > ---
> > drivers/iio/magnetometer/hmc5843_i2c.c | 7 ++++++-
> > drivers/iio/magnetometer/hmc5843_spi.c | 7 ++++++-
> > 2 files changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/iio/magnetometer/hmc5843_i2c.c b/drivers/iio/magnetometer/hmc5843_i2c.c
> > index 3de7f4426ac4..86abba5827a2 100644
> > --- a/drivers/iio/magnetometer/hmc5843_i2c.c
> > +++ b/drivers/iio/magnetometer/hmc5843_i2c.c
> > @@ -58,8 +58,13 @@ static const struct regmap_config hmc5843_i2c_regmap_config = {
> > static int hmc5843_i2c_probe(struct i2c_client *cli,
> > const struct i2c_device_id *id)
> > {
> > + struct regmap *regmap = devm_regmap_init_i2c(cli,
> > + &hmc5843_i2c_regmap_config);
> > + if (IS_ERR(regmap))
> > + return PTR_ERR(regmap);
> > +
> > return hmc5843_common_probe(&cli->dev,
> > - devm_regmap_init_i2c(cli, &hmc5843_i2c_regmap_config),
> > + regmap,
> > id->driver_data, id->name);
> > }
> >
> > diff --git a/drivers/iio/magnetometer/hmc5843_spi.c b/drivers/iio/magnetometer/hmc5843_spi.c
> > index 535f03a70d63..8355713651d4 100644
> > --- a/drivers/iio/magnetometer/hmc5843_spi.c
> > +++ b/drivers/iio/magnetometer/hmc5843_spi.c
> > @@ -58,6 +58,7 @@ static const struct regmap_config hmc5843_spi_regmap_config = {
> > static int hmc5843_spi_probe(struct spi_device *spi)
> > {
> > int ret;
> > + struct regmap *regmap;
> > const struct spi_device_id *id = spi_get_device_id(spi);
> >
> > spi->mode = SPI_MODE_3;
> > @@ -67,8 +68,12 @@ static int hmc5843_spi_probe(struct spi_device *spi)
> > if (ret)
> > return ret;
> >
> > + regmap = devm_regmap_init(spi, &hmc5843_spi_regmap_config);

devm_regmap_init_spi


> > + if (IS_ERR(regmap))
> > + return PTR_ERR(devm_regmap);

It's called regmap.

> > +
> > return hmc5843_common_probe(&spi->dev,
> > - devm_regmap_init_spi(spi, &hmc5843_spi_regmap_config),
> > + regmap,
> > id->driver_data, id->name);
> > }
> >
>