Skip to contentSkip to conversation

drm/bridge: sii902x: re-order conditions to prevent out of bounds read

PATCH Applied Dan Carpenter 3 participants 5 messages Jun 7, 2019

Subsystems (4)
DRM drivers DRM misc Kernel janitors DRM drivers for bridge chips
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/2 drm/bridge: sii902x: re-order conditions to prevent out of bounds read

This should check that "i" is within bounds before checking reading from the array.

Fixes ff5781634c41 ("drm/bridge: sii902x: Implement HDMI audio support")
Signed-off-by Dan Carpenter <[email protected]>
---
 drivers/gpu/drm/bridge/sii902x.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/gpu/drm/bridge/sii902x.c +2 -2
index d6f98d388ac2..6b03616d6bc3 100644
@@ -589,8 +589,8 @@ static int sii902x_audio_hw_params(struct device *dev, void *data,
 	if (ret)
 		goto out;
 
-	for (i = 0; sii902x->audio.i2s_fifo_sequence[i] &&
-		     i < ARRAY_SIZE(sii902x->audio.i2s_fifo_sequence); i++)
+	for (i = 0; i < ARRAY_SIZE(sii902x->audio.i2s_fifo_sequence) &&
+		    sii902x->audio.i2s_fifo_sequence[i]; i++)
 		regmap_write(sii902x->regmap,
 			     SII902X_TPI_I2S_ENABLE_MAPPING_REG,
 			     sii902x->audio.i2s_fifo_sequence[i]);
Signature · 1 line
2.20.1
#1 lore raw

Am 07.06.2019 09:27, schrieb Dan Carpenter:

Quoted 14 lines

This should check that "i" is within bounds before checking reading from the array.

Fixes ff5781634c41 ("drm/bridge: sii902x: Implement HDMI audio support")
Signed-off-by Dan Carpenter <[email protected]>
---
 drivers/gpu/drm/bridge/sii902x.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/gpu/drm/bridge/sii902x.c +0 -0
index d6f98d388ac2..6b03616d6bc3 100644

drivers/gpu/drm/bridge/sii902x.c static int sii902x_audio_hw_params(struct device *dev, void *data, · line 596

Quoted 10 lines
drivers/gpu/drm/bridge/sii902x.c +2 -2
@@ -589,8 +589,8 @@ static int sii902x_audio_hw_params(struct device *dev, void *data,
 	if (ret)
 		goto out;
 
-	for (i = 0; sii902x->audio.i2s_fifo_sequence[i] &&
-		     i < ARRAY_SIZE(sii902x->audio.i2s_fifo_sequence); i++)
+	for (i = 0; i < ARRAY_SIZE(sii902x->audio.i2s_fifo_sequence) &&
+		    sii902x->audio.i2s_fifo_sequence[i]; i++)
 		regmap_write(sii902x->regmap,
 			     SII902X_TPI_I2S_ENABLE_MAPPING_REG,
 			     sii902x->audio.i2s_fifo_sequence[i]);

mmmh, i am a big fan of KISS and in this case i would check sii902x->audio.i2s_fifo_sequence[i] outside for(). like:

	 for (i = 0; i < ARRAY_SIZE(sii902x->audio.i2s_fifo_sequence); i++) {
	        if (!sii902x->audio.i2s_fifo_sequence[i])
                         break;

                 regmap_write(sii902x->regmap,
  			     SII902X_TPI_I2S_ENABLE_MAPPING_REG,
  			     sii902x->audio.i2s_fifo_sequence[i]);
	}

just my 2 cents,

re,
wh

author
#2 lore raw
Quoted 32 lines · walter harms

Am 07.06.2019 09:27, schrieb Dan Carpenter:

Quoted 14 lines

This should check that "i" is within bounds before checking reading from the array.

Fixes ff5781634c41 ("drm/bridge: sii902x: Implement HDMI audio support")
Signed-off-by Dan Carpenter <[email protected]>
---
 drivers/gpu/drm/bridge/sii902x.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/gpu/drm/bridge/sii902x.c +0 -0
index d6f98d388ac2..6b03616d6bc3 100644

drivers/gpu/drm/bridge/sii902x.c static int sii902x_audio_hw_params(struct device *dev, void *data, · line 596

Quoted 10 lines
drivers/gpu/drm/bridge/sii902x.c +2 -2
@@ -589,8 +589,8 @@ static int sii902x_audio_hw_params(struct device *dev, void *data,
 	if (ret)
 		goto out;
 
-	for (i = 0; sii902x->audio.i2s_fifo_sequence[i] &&
-		     i < ARRAY_SIZE(sii902x->audio.i2s_fifo_sequence); i++)
+	for (i = 0; i < ARRAY_SIZE(sii902x->audio.i2s_fifo_sequence) &&
+		    sii902x->audio.i2s_fifo_sequence[i]; i++)
 		regmap_write(sii902x->regmap,
 			     SII902X_TPI_I2S_ENABLE_MAPPING_REG,
 			     sii902x->audio.i2s_fifo_sequence[i]);

mmmh, i am a big fan of KISS and in this case i would check sii902x->audio.i2s_fifo_sequence[i] outside for(). like:

Quoted 3 lines
	 for (i = 0; i < ARRAY_SIZE(sii902x->audio.i2s_fifo_sequence); i++) {
	        if (!sii902x->audio.i2s_fifo_sequence[i])
                         break;

That's probably a better way, but either way is acceptable and I left it how the author wrote it.

regards,
dan carpenter

Reviewed-by on v1 1/2
#3 lore raw
In reply to Dan Carpenter
Quoted 5 lines · Dan Carpenter

This should check that "i" is within bounds before checking reading from the array.

Fixes ff5781634c41 ("drm/bridge: sii902x: Implement HDMI audio support")
Signed-off-by Dan Carpenter <[email protected]>
Reviewed-by Andrzej Hajda <[email protected]>

 --
Regards
Andrzej

Quoted 19 lines
---
 drivers/gpu/drm/bridge/sii902x.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/gpu/drm/bridge/sii902x.c +2 -2
index d6f98d388ac2..6b03616d6bc3 100644
@@ -589,8 +589,8 @@ static int sii902x_audio_hw_params(struct device *dev, void *data,
 	if (ret)
 		goto out;
 
-	for (i = 0; sii902x->audio.i2s_fifo_sequence[i] &&
-		     i < ARRAY_SIZE(sii902x->audio.i2s_fifo_sequence); i++)
+	for (i = 0; i < ARRAY_SIZE(sii902x->audio.i2s_fifo_sequence) &&
+		    sii902x->audio.i2s_fifo_sequence[i]; i++)
 		regmap_write(sii902x->regmap,
 			     SII902X_TPI_I2S_ENABLE_MAPPING_REG,
 			     sii902x->audio.i2s_fifo_sequence[i]);
Applied
#4 lore raw
In reply to Dan Carpenter
Quoted 14 lines · Dan Carpenter

This should check that "i" is within bounds before checking reading from the array.

Fixes ff5781634c41 ("drm/bridge: sii902x: Implement HDMI audio support")
Signed-off-by Dan Carpenter <[email protected]>
---
 drivers/gpu/drm/bridge/sii902x.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/gpu/drm/bridge/sii902x.c +0 -0
index d6f98d388ac2..6b03616d6bc3 100644

drivers/gpu/drm/bridge/sii902x.c static int sii902x_audio_hw_params(struct device *dev, void *data, · line 596

Quoted 10 lines
drivers/gpu/drm/bridge/sii902x.c +2 -2
@@ -589,8 +589,8 @@ static int sii902x_audio_hw_params(struct device *dev, void *data,
 	if (ret)
 		goto out;
 
-	for (i = 0; sii902x->audio.i2s_fifo_sequence[i] &&
-		     i < ARRAY_SIZE(sii902x->audio.i2s_fifo_sequence); i++)
+	for (i = 0; i < ARRAY_SIZE(sii902x->audio.i2s_fifo_sequence) &&
+		    sii902x->audio.i2s_fifo_sequence[i]; i++)
 		regmap_write(sii902x->regmap,
 			     SII902X_TPI_I2S_ENABLE_MAPPING_REG,
 			     sii902x->audio.i2s_fifo_sequence[i]);

Queued to drm-misc-next.

--
Regards
Andrzej