[PATCH] media: i2c: adv7604: fix out-of-bounds access with a one-block EDID

[Guo Zihao]

When VIDIOC_S_EDID is called with a one-block EDID (no CTA extension
block, hence no Source Physical Address), v4l2_get_edid_phys_addr()
leaves spa_loc at 0. The fallback path then sets spa_loc to 128 and
reads edid->edid[128] and edid->edid[129], 2 bytes past the end of
the 128-byte kernel buffer that check_array_args() allocated for the
EDID data. The physical address is subsequently written back at the
same out-of-bounds location, so this is both a 2-byte out-of-bounds
read and write of kernel heap memory.

This regressed when support for EDIDs up to 4 blocks was added:
commit 54b74981c86c ("media: adv7604: writing a one-block EDID
failed") carefully handled the one-block case, but the fallback that
was added six days later undid that protection again.

Only take the spa_loc = 128 fallback if a second block exists, and
only write the physical address back into the EDID if a SPA location
was found. This restores the one-block behavior of 54b74981c86c.

Fixes: c730ff32b917 ("media: adv7604: support EDIDs up to 4 blocks")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Guo Zihao <guozh23@xxxxxxxxxxxx>
---

Triggering this requires nothing more than a VIDIOC_S_EDID call with a
single-block EDID on a subdev node, i.e. the most basic EDID a
userspace application can pass.

Verified by an ASAN-based userspace reproduction of the
check_array_args()/adv76xx_set_edid() path (kmalloc(128) buffer,
blocks=1, no SPA):

    ERROR: AddressSanitizer: heap-buffer-overflow
    READ of size 1 ... 0 bytes to the right of 128-byte region

With this fix applied the reproduction is clean. No regressions for
multi-block EDIDs containing a SPA: the spa_loc != 0 paths are
unchanged.

 drivers/media/i2c/adv7604.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index ac9c69ce438f..c007ac0bd8e5 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -2358,7 +2358,7 @@ static int adv76xx_set_edid(struct v4l2_subdev *sd, struct v4l2_edid *edid)
 	if (err)
 		return err;
 
-	if (!spa_loc) {
+	if (!spa_loc && edid->blocks > 1) {
 		/*
 		 * There is no SPA, so just set spa_loc to 128 and pa to whatever
 		 * data is there.
@@ -2404,8 +2404,10 @@ static int adv76xx_set_edid(struct v4l2_subdev *sd, struct v4l2_edid *edid)
 				  mask, (spa_loc & 0x100) ? mask : 0);
 	}
 
-	edid->edid[spa_loc] = state->spa_port_a[0];
-	edid->edid[spa_loc + 1] = state->spa_port_a[1];
+	if (spa_loc) {
+		edid->edid[spa_loc] = state->spa_port_a[0];
+		edid->edid[spa_loc + 1] = state->spa_port_a[1];
+	}
 
 	memcpy(state->edid.edid, edid->edid, 128 * edid->blocks);
 	state->edid.blocks = edid->blocks;
-- 
2.50.1