iio: light: Add check for array bounds in veml6075_read_int_time_ms

PATCH In review Karan Sanghavi 3 participants 6 messages Feb 20, 2025 – Mar 8, 2025

Subsystems (2)

IIO Vishay VEML6075 uva and uvb light sensor driver

lore

Share & tools

lore

Karan Sanghavi

author

2025-02-20 17:34 UTC #0 lore raw

PATCH v3 1/1 iio: light: Add check for array bounds in veml6075_read_int_time_ms

The array contains only 5 elements, but the index calculated by veml6075_read_int_time_index can range from 0 to 7,
which could lead to out-of-bounds access. The check prevents this issue.

Coverity Issue
CID 1574309: (#1 of 1): Out-of-bounds read (OVERRUN)
overrun-local: Overrunning array veml6075_it_ms of 5 4-byte elements at element index 7 (byte offset 31) using index int_index (which evaluates to 7)

Fixes 3b82f43238ae ("iio: light: add VEML6075 UVA and UVB light sensor driver")

Signed-off-by Karan Sanghavi <[email protected]>

---
 drivers/iio/light/veml6075.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

drivers/iio/light/veml6075.c +6 -1

index 05d4c0e9015d..5dd951f6e989 100644
@@ -201,7 +201,12 @@ static int veml6075_read_int_time_index(struct veml6075_data *data)
 	if (ret < 0)
 		return ret;
 
-	return FIELD_GET(VEML6075_CONF_IT, conf);
+	int int_index = FIELD_GET(VEML6075_CONF_IT, conf);
+
+	if (int_index >= ARRAY_SIZE(veml6075_it_ms))
+		return -EINVAL;
+
+	return int_index;
 }
 
 static int veml6075_read_int_time_ms(struct veml6075_data *data, int *val)

Signature · 1 line

2.43.0

Javier Carrasco

maintainer Reviewed-by on v3 1/1

2025-02-20 20:00 UTC #1 lore raw

Quoted 21 lines · Karan Sanghavi

The array contains only 5 elements, but the index calculated by veml6075_read_int_time_index can range from 0 to 7,
which could lead to out-of-bounds access. The check prevents this issue.
Coverity Issue
CID 1574309: (#1 of 1): Out-of-bounds read (OVERRUN)
overrun-local: Overrunning array veml6075_it_ms of 5 4-byte elements at element index 7 (byte offset 31) using index int_index (which evaluates to 7)
Fixes 3b82f43238ae ("iio: light: add VEML6075 UVA and UVB light sensor driver")
Signed-off-by Karan Sanghavi <[email protected]>
---
 drivers/iio/light/veml6075.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/iio/light/veml6075.c +0 -0
index 05d4c0e9015d..5dd951f6e989 100644

drivers/iio/light/veml6075.c static int veml6075_read_int_time_index(struct veml6075_data *data) · line 204

Quoted 4 lines

drivers/iio/light/veml6075.c +0 -1

@@ -201,7 +201,12 @@ static int veml6075_read_int_time_index(struct veml6075_data *data)
 	if (ret < 0)
 		return ret;
 
-	return FIELD_GET(VEML6075_CONF_IT, conf);

Please declare the variable at the beginning of the function (there are some integers there already) and rename it to follow the driver convention, it for integration time: it_index

drivers/iio/light/veml6075.c static int veml6075_read_int_time_index(struct veml6075_data *data) · line 212

Quoted 9 lines

drivers/iio/light/veml6075.c +6 -0

+	int int_index = FIELD_GET(VEML6075_CONF_IT, conf);
+
+	if (int_index >= ARRAY_SIZE(veml6075_it_ms))
+		return -EINVAL;
+
+	return int_index;
 }
 
 static int veml6075_read_int_time_ms(struct veml6075_data *data, int *val)

With that applied:

Reviewed-by Javier Carrasco <[email protected]>

Javier Carrasco

maintainer Reviewed-by

2025-02-20 20:02 UTC #2 lore raw

Quoted 39 lines · Javier Carrasco

Quoted 21 lines · Karan Sanghavi
The array contains only 5 elements, but the index calculated by veml6075_read_int_time_index can range from 0 to 7,
which could lead to out-of-bounds access. The check prevents this issue.
Coverity Issue
CID 1574309: (#1 of 1): Out-of-bounds read (OVERRUN)
overrun-local: Overrunning array veml6075_it_ms of 5 4-byte elements at element index 7 (byte offset 31) using index int_index (which evaluates to 7)
Fixes 3b82f43238ae ("iio: light: add VEML6075 UVA and UVB light sensor driver")
Signed-off-by Karan Sanghavi <[email protected]>
---
 drivers/iio/light/veml6075.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/iio/light/veml6075.c +0 -0
index 05d4c0e9015d..5dd951f6e989 100644
drivers/iio/light/veml6075.c static int veml6075_read_int_time_index(struct veml6075_data *data) · line 204
Quoted 4 lines
drivers/iio/light/veml6075.c +0 -1
@@ -201,7 +201,12 @@ static int veml6075_read_int_time_index(struct veml6075_data *data)
 	if (ret < 0)
 		return ret;
 
-	return FIELD_GET(VEML6075_CONF_IT, conf);
Please declare the variable at the beginning of the function (there are some integers there already) and rename it to follow the driver convention, it for integration time: it_index
Quoted 8 lines
drivers/iio/light/veml6075.c +6 -0
+	int int_index = FIELD_GET(VEML6075_CONF_IT, conf);
+
+	if (int_index >= ARRAY_SIZE(veml6075_it_ms))
+		return -EINVAL;
+
+	return int_index;
 }
 

Quoted 5 lines

drivers/iio/light/veml6075.c static int veml6075_read_int_time_index(struct veml6075_data *data) · line 212
Quoted 1 line
drivers/iio/light/veml6075.c +0 -0
 static int veml6075_read_int_time_ms(struct veml6075_data *data, int *val)
With that applied:
Reviewed-by Javier Carrasco <[email protected]>

Fix:

Reviewed-by Javier Carrasco <[email protected]>

Jonathan Cameron

maintainer

2025-02-22 16:43 UTC #3 lore raw

In reply to Karan Sanghavi

Quoted 10 lines · On Thu, 20 Feb 2025 17:34:36 +0000 Karan Sanghavi <[email protected]>…

The array contains only 5 elements, but the index calculated by veml6075_read_int_time_index can range from 0 to 7,
which could lead to out-of-bounds access. The check prevents this issue.
Coverity Issue
CID 1574309: (#1 of 1): Out-of-bounds read (OVERRUN)
overrun-local: Overrunning array veml6075_it_ms of 5 4-byte elements at element index 7 (byte offset 31) using index int_index (which evaluates to 7)

Quoted 3 lines

Fixes 3b82f43238ae ("iio: light: add VEML6075 UVA and UVB light sensor driver")
Signed-off-by Karan Sanghavi <[email protected]>
---

Superficially this looks hardening against malicious or broken hardware. That is fine to add, but not worth backporting or (in my opinion) adding a fixes tag.

Quoted 21 lines

 drivers/iio/light/veml6075.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

drivers/iio/light/veml6075.c +6 -1

index 05d4c0e9015d..5dd951f6e989 100644
@@ -201,7 +201,12 @@ static int veml6075_read_int_time_index(struct veml6075_data *data)
 	if (ret < 0)
 		return ret;
 
-	return FIELD_GET(VEML6075_CONF_IT, conf);
+	int int_index = FIELD_GET(VEML6075_CONF_IT, conf);
+
+	if (int_index >= ARRAY_SIZE(veml6075_it_ms))
+		return -EINVAL;
+
+	return int_index;
 }
 
 static int veml6075_read_int_time_ms(struct veml6075_data *data, int *val)

Jonathan Cameron

maintainer

2025-02-22 16:45 UTC #4 lore raw

Quoted 13 lines · On Sat, 22 Feb 2025 16:43:37 +0000 Jonathan Cameron <[email protected]> wrote:

Quoted 10 lines · On Thu, 20 Feb 2025 17:34:36 +0000 Karan Sanghavi <[email protected]>…
The array contains only 5 elements, but the index calculated by veml6075_read_int_time_index can range from 0 to 7,
which could lead to out-of-bounds access. The check prevents this issue.
Coverity Issue
CID 1574309: (#1 of 1): Out-of-bounds read (OVERRUN)
overrun-local: Overrunning array veml6075_it_ms of 5 4-byte elements at element index 7 (byte offset 31) using index int_index (which evaluates to 7)

Quoted 6 lines

Quoted 3 lines
Fixes 3b82f43238ae ("iio: light: add VEML6075 UVA and UVB light sensor driver")
Signed-off-by Karan Sanghavi <[email protected]>
---
Superficially this looks hardening against malicious or broken hardware. That is fine to add, but not worth backporting or (in my opinion) adding a fixes tag.

Ah I see Javier asked for the fixes tag. Ok. Maybe just add a note that the hardware is not expected to return such an out of bounds value.

That will help me to remember we don't need to rush this one upstream!

Jonathan

Quoted 23 lines

Quoted 22 lines

 drivers/iio/light/veml6075.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

drivers/iio/light/veml6075.c +6 -1

index 05d4c0e9015d..5dd951f6e989 100644
@@ -201,7 +201,12 @@ static int veml6075_read_int_time_index(struct veml6075_data *data)
 	if (ret < 0)
 		return ret;
 
-	return FIELD_GET(VEML6075_CONF_IT, conf);
+	int int_index = FIELD_GET(VEML6075_CONF_IT, conf);
+
+	if (int_index >= ARRAY_SIZE(veml6075_it_ms))
+		return -EINVAL;
+
+	return int_index;
 }
 
 static int veml6075_read_int_time_ms(struct veml6075_data *data, int *val)

Jonathan Cameron

maintainer

2025-03-08 16:50 UTC #5 lore raw

Quoted 19 lines · On Sat, 22 Feb 2025 16:45:19 +0000 Jonathan Cameron <[email protected]> wrote:

Quoted 16 lines · On Sat, 22 Feb 2025 16:43:37 +0000 Jonathan Cameron <[email protected]> wrote:
Quoted 13 lines · On Thu, 20 Feb 2025 17:34:36 +0000 Karan Sanghavi <[email protected]>…
The array contains only 5 elements, but the index calculated by veml6075_read_int_time_index can range from 0 to 7,
which could lead to out-of-bounds access. The check prevents this issue.
Coverity Issue
CID 1574309: (#1 of 1): Out-of-bounds read (OVERRUN)
overrun-local: Overrunning array veml6075_it_ms of 5 4-byte elements at element index 7 (byte offset 31) using index int_index (which evaluates to 7)
Fixes 3b82f43238ae ("iio: light: add VEML6075 UVA and UVB light sensor driver")
Signed-off-by Karan Sanghavi <[email protected]>
---

Quoted 7 lines

Quoted 3 lines
Superficially this looks hardening against malicious or broken hardware. That is fine to add, but not worth backporting or (in my opinion) adding a fixes tag.
Ah I see Javier asked for the fixes tag. Ok. Maybe just add a note that the hardware is not expected to return such an out of bounds value.
That will help me to remember we don't need to rush this one upstream!

I guess you don't have time so I made the changes requested and applied. (mostly so I can stop tracking the status!)

Jonathan

Quoted 28 lines

Jonathan

Quoted 23 lines

Quoted 22 lines

  
 drivers/iio/light/veml6075.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

drivers/iio/light/veml6075.c +6 -1

index 05d4c0e9015d..5dd951f6e989 100644
@@ -201,7 +201,12 @@ static int veml6075_read_int_time_index(struct veml6075_data *data)
 	if (ret < 0)
 		return ret;
 
-	return FIELD_GET(VEML6075_CONF_IT, conf);
+	int int_index = FIELD_GET(VEML6075_CONF_IT, conf);
+
+	if (int_index >= ARRAY_SIZE(veml6075_it_ms))
+		return -EINVAL;
+
+	return int_index;
 }
 
 static int veml6075_read_int_time_ms(struct veml6075_data *data, int *val)