Skip to contentSkip to conversation

iio: dac: mcp47feb02: reject out-of-range raw values

PATCH Superseded David Carlier 3 participants 5 messages Oct 24 – Oct 25 2 messages not in the archive

Subsystems (2)
Stable branch IIO

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

David Carlier

author
#0 lore raw

PATCH 1/1 iio: dac: mcp47feb02: reject out-of-range raw values

mcp47feb02_write_raw() writes the user supplied raw value to the DAC register without checking it against the chip resolution. The parts are 8, 10 or 12 bit, and the bits above the resolution are not implemented, so writing 300 to out_voltage0_raw on an 8-bit part sets the output to code 44 instead of failing, and -1 is written as 0xffff.

The out-of-range value is also cached in dac_data, so it is written back on resume and stored to EEPROM by store_eeprom.

Return -EINVAL for values outside [0, 2^resolution - 1].

Found by code inspection. Build tested with W=1 (=m and =y) only; no hardware was available for runtime testing.

Fixes bf394cc80369 ("iio: dac: adding support for Microchip MCP47FEB02")
Cc [email protected]
Assisted-by LLM
Signed-off-by David Carlier <[email protected]>
---
 drivers/iio/dac/mcp47feb02-core.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/iio/dac/mcp47feb02-core.c +3 -0
index 88bffbab0854..1a0e80b5db87 100644
@@ -696,6 +696,9 @@ static int mcp47feb02_write_raw(struct iio_dev *indio_dev, struct iio_chan_spec
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
+		if (val < 0 || val >= BIT(data->chip_features->resolution))
+			return -EINVAL;
+
 		ret = regmap_write(data->regmap, REG_ADDR(ch->address), val);
 		if (ret)
 			return ret;
Signature · 1 line
2.55.0
Reviewed-by on v1 1/1
#1 lore raw
Quoted 6 lines · On Sat, 24 Oct 2026 22:44:23 +0100 David Carlier <[email protected]> wrote:

mcp47feb02_write_raw() writes the user supplied raw value to the DAC register without checking it against the chip resolution. The parts are 8, 10 or 12 bit, and the bits above the resolution are not implemented, so writing 300 to out_voltage0_raw on an 8-bit part sets the output to code 44 instead of failing, and -1 is written as 0xffff.

Quoted 7 lines

The out-of-range value is also cached in dac_data, so it is written back on resume and stored to EEPROM by store_eeprom.

Return -EINVAL for values outside [0, 2^resolution - 1].

Found by code inspection. Build tested with W=1 (=m and =y) only; no hardware was available for runtime testing.

This should go under the ---.

Quoted 3 lines
Fixes bf394cc80369 ("iio: dac: adding support for Microchip MCP47FEB02")
Cc [email protected]
Assisted-by LLM

This tag has been deprecated.

Otherwise LGTM.

Reviewed-by Joshua Crofts <[email protected]>
Signature · 2 lines
Kind regards,
Joshua Crofts
#2 lore raw
In reply to David Carlier
Quoted 13 lines · David Carlier

mcp47feb02_write_raw() writes the user supplied raw value to the DAC register without checking it against the chip resolution. The parts are 8, 10 or 12 bit, and the bits above the resolution are not implemented, so writing 300 to out_voltage0_raw on an 8-bit part sets the output to code 44 instead of failing, and -1 is written as 0xffff.

The out-of-range value is also cached in dac_data, so it is written back on resume and stored to EEPROM by store_eeprom.

Return -EINVAL for values outside [0, 2^resolution - 1].

Found by code inspection. Build tested with W=1 (=m and =y) only; no hardware was available for runtime testing.

...

drivers/iio/dac/mcp47feb02-core.c static int mcp47feb02_write_raw(struct iio_dev *indio_dev, struct iio_chan_spec · line 699

Quoted 2 lines
drivers/iio/dac/mcp47feb02-core.c +1 -0
 	case IIO_CHAN_INFO_RAW:
+		if (val < 0 || val >= BIT(data->chip_features->resolution))

If resolution is 32, this will suck. (Yes, this is a bit theoretical case.)

drivers/iio/dac/mcp47feb02-core.c static int mcp47feb02_write_raw(struct iio_dev *indio_dev, struct iio_chan_spec · line 700

Quoted 1 line
drivers/iio/dac/mcp47feb02-core.c +1 -0
+			return -EINVAL;

Not sure about this error code. Do other IIO drivers use same in the similar situation?

Signature · 2 lines
With Best Regards,
Andy Shevchenko

A message that is not in the archive (sent off-list or not yet received): CA+XhMqx5jusLqo8G2XCY9gy_P=3-jAwTacgzrR4XFyjpagi8dA@mail.gmail.com

#4 lore raw
Quoted 3 lines · David CARLIER
Quoted 1 line · Andy Shevchenko

On Sat, Oct 24, 2026 at 10:44:23PM +0100, David Carlier wrote:

...

Quoted 6 lines
Quoted 5 lines
Quoted 2 lines

case IIO_CHAN_INFO_RAW:
+ if (val < 0 || val >= BIT(data->chip_features->resolution))

If resolution is 32, this will suck. (Yes, this is a bit theoretical case.)

Quoted 8 lines

Hmm true.

Quoted 4 lines
Quoted 1 line

+ return -EINVAL;

Not sure about this error code. Do other IIO drivers use same in the similar situation?

Yes other drivers for every out-of-range check IIO_CHAN_INFO_RAW.

No doubts. But I asked a different question. Can you answer it?

Signature · 2 lines
With Best Regards,
Andy Shevchenko

A message that is not in the archive (sent off-list or not yet received): CA+XhMqzZZyN80nP2ER3jyTsM9UC2xMRLiqdCsAtp+K=hDbqtQg@mail.gmail.com

#6 lore raw
Quoted 6 lines · David CARLIER
Quoted 4 lines · Andy Shevchenko
Quoted 3 lines · David CARLIER
Quoted 1 line · Andy Shevchenko

On Sat, Oct 24, 2026 at 10:44:23PM +0100, David Carlier wrote:

...

Quoted 10 lines
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Quoted 1 line

+ return -EINVAL;

Not sure about this error code. Do other IIO drivers use same in the similar situation?

Yes other drivers for every out-of-range check IIO_CHAN_INFO_RAW.

No doubts. But I asked a different question. Can you answer it?

I meant in this case the other drivers -EINVAL.

OK, thanks for confirming!

Signature · 2 lines
With Best Regards,
Andy Shevchenko