iio: mcp320x: Fix NULL pointer dereference

PATCH Applied Manfred Schlaegl 3 participants 5 messages Jul 10, 2015 – Jul 19, 2015 IIO

lore

Share & tools

lore

Manfred Schlaegl

author

2015-07-10 20:55 UTC #0 lore raw

PATCH 1/1 iio: mcp320x: Fix NULL pointer dereference

On reading in_voltage_scale of we got an NULL pointer dereference Oops.

The reason for this is, that mcp320x_read_raw tries to access chip_info->resolution from struct mcp320x, but chip_info is never set.

chip_info was never set since the driver was added, but there was no acute problem, because it was not referenced.
The acute problem exists since b12206e917ac34bec41b9ff93d37d8bd53a2b3bc
iio: adc: mcp320x. Add support for more ADCs

This patch fixes the issue by setting chip_info in mcp320x_probe.

Signed-off-by Manfred Schlaegl <[email protected]>

---
 drivers/iio/adc/mcp320x.c |    2 ++
 1 file changed, 2 insertions(+)

drivers/iio/adc/mcp320x.c +2 -0

index 8d9c9b9..d819823 100644
@@ -299,6 +299,8 @@ static int mcp320x_probe(struct spi_device *spi)
 	indio_dev->channels = chip_info->channels;
 	indio_dev->num_channels = chip_info->num_channels;
 
+	adc->chip_info = chip_info;
+
 	adc->transfer[0].tx_buf = &adc->tx_buf;
 	adc->transfer[0].len = sizeof(adc->tx_buf);
 	adc->transfer[1].rx_buf = adc->rx_buf;

Signature · 1 line

1.7.10.4

Michael Welling

Reviewed-by on v1 1/1

2015-07-13 14:16 UTC #1 lore raw

Quoted 23 lines · Manfred Schlaegl

On reading in_voltage_scale of we got an NULL pointer dereference Oops.
The reason for this is, that mcp320x_read_raw tries to access chip_info->resolution from struct mcp320x, but chip_info is never set.
chip_info was never set since the driver was added, but there was no acute problem, because it was not referenced.
The acute problem exists since b12206e917ac34bec41b9ff93d37d8bd53a2b3bc
iio: adc: mcp320x. Add support for more ADCs
This patch fixes the issue by setting chip_info in mcp320x_probe.
Signed-off-by Manfred Schlaegl <[email protected]>
---
 drivers/iio/adc/mcp320x.c |    2 ++
 1 file changed, 2 insertions(+)
drivers/iio/adc/mcp320x.c +0 -0
index 8d9c9b9..d819823 100644

drivers/iio/adc/mcp320x.c static int mcp320x_probe(struct spi_device *spi) · line 303

Quoted 5 lines

drivers/iio/adc/mcp320x.c +2 -0

@@ -299,6 +299,8 @@ static int mcp320x_probe(struct spi_device *spi)
 	indio_dev->channels = chip_info->channels;
 	indio_dev->num_channels = chip_info->num_channels;
 
+	adc->chip_info = chip_info;
+

Looks good to me.

Reviewed-by Michael Welling <[email protected]>

Quoted 6 lines

drivers/iio/adc/mcp320x.c +0 -0

 	adc->transfer[0].tx_buf = &adc->tx_buf;
 	adc->transfer[0].len = sizeof(adc->tx_buf);
 	adc->transfer[1].rx_buf = adc->rx_buf;

Signature · 1 line

1.7.10.4

Manfred Schlaegl

author

2015-07-14 11:53 UTC #2 lore raw

Quoted 27 lines · Michael Welling

Quoted 26 lines · Manfred Schlaegl
On reading in_voltage_scale of we got an NULL pointer dereference Oops.
The reason for this is, that mcp320x_read_raw tries to access chip_info->resolution from struct mcp320x, but chip_info is never set.
chip_info was never set since the driver was added, but there was no acute problem, because it was not referenced.
The acute problem exists since b12206e917ac34bec41b9ff93d37d8bd53a2b3bc
iio: adc: mcp320x. Add support for more ADCs
This patch fixes the issue by setting chip_info in mcp320x_probe.
Signed-off-by Manfred Schlaegl <[email protected]>
---
 drivers/iio/adc/mcp320x.c |    2 ++
 1 file changed, 2 insertions(+)
drivers/iio/adc/mcp320x.c +0 -0
index 8d9c9b9..d819823 100644
@@ -299,6 +299,8 @@ static int mcp320x_probe(struct spi_device *spi)
 	indio_dev->channels = chip_info->channels;
 	indio_dev->num_channels = chip_info->num_channels;
 

Quoted 13 lines

drivers/iio/adc/mcp320x.c static int mcp320x_probe(struct spi_device *spi) · line 303
Quoted 2 lines
drivers/iio/adc/mcp320x.c +2 -0
+	adc->chip_info = chip_info;
+
Looks good to me.
Reviewed-by Michael Welling <[email protected]>
Quoted 6 lines
drivers/iio/adc/mcp320x.c +0 -0
 	adc->transfer[0].tx_buf = &adc->tx_buf;
 	adc->transfer[0].len = sizeof(adc->tx_buf);
 	adc->transfer[1].rx_buf = adc->rx_buf;
Signature · 1 line
1.7.10.4

Thanks for reviewing!

best regards,
manfred

Jonathan Cameron

maintainer Applied

2015-07-19 13:00 UTC #3 lore raw

Quoted 42 lines · Manfred Schlaegl

Quoted 27 lines · Michael Welling
Quoted 26 lines · Manfred Schlaegl
On reading in_voltage_scale of we got an NULL pointer dereference Oops.
The reason for this is, that mcp320x_read_raw tries to access chip_info->resolution from struct mcp320x, but chip_info is never set.
chip_info was never set since the driver was added, but there was no acute problem, because it was not referenced.
The acute problem exists since b12206e917ac34bec41b9ff93d37d8bd53a2b3bc
iio: adc: mcp320x. Add support for more ADCs
This patch fixes the issue by setting chip_info in mcp320x_probe.
Signed-off-by Manfred Schlaegl <[email protected]>
---
 drivers/iio/adc/mcp320x.c |    2 ++
 1 file changed, 2 insertions(+)
drivers/iio/adc/mcp320x.c +0 -0
index 8d9c9b9..d819823 100644
@@ -299,6 +299,8 @@ static int mcp320x_probe(struct spi_device *spi)
 	indio_dev->channels = chip_info->channels;
 	indio_dev->num_channels = chip_info->num_channels;
 
Quoted 13 lines
drivers/iio/adc/mcp320x.c static int mcp320x_probe(struct spi_device *spi) · line 303
Quoted 2 lines
drivers/iio/adc/mcp320x.c +2 -0
+	adc->chip_info = chip_info;
+
Looks good to me.
Reviewed-by Michael Welling <[email protected]>
Quoted 6 lines
drivers/iio/adc/mcp320x.c +0 -0
 	adc->transfer[0].tx_buf = &adc->tx_buf;
 	adc->transfer[0].len = sizeof(adc->tx_buf);
 	adc->transfer[1].rx_buf = adc->rx_buf;
Signature · 1 line
1.7.10.4

Quoted 5 lines

Thanks for reviewing!
best regards,
manfred

Applied to the fixes-togreg branch of iio.git

Michael, there are moves afoot to clarify the meaning of reviewed-by as covering only more substantial reviews (kind of tricky on a one liner like this!) Hence for similar patches in future, an Acked-by may be more appropriate.

Thanks,

Jonathan

Michael Welling

2015-07-19 23:56 UTC #4 lore raw

Quoted 7 lines · Jonathan Cameron

Applied to the fixes-togreg branch of iio.git
Michael, there are moves afoot to clarify the meaning of reviewed-by as covering only more substantial reviews (kind of tricky on a one liner like this!) Hence for similar patches in future, an Acked-by may be more appropriate.

Actually I did review the patch and sent comments to the author.

In my haste I pressed reply istead of reply-all.. Oooops.

Quoted 3 lines

Thanks,
Jonathan