linux-kernel - [PATCH 08/11] media: adv7842: better document EDID block size

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <6bed7a69367856080a62e3ee89df6a2a3d0d5f20.1623846327.git.mchehab+huawei@kernel.org>
Date:   Wed, 16 Jun 2021 14:28:34 +0200
From:   Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
To:     unlisted-recipients:; (no To-header on input)
Cc:     linuxarm@...wei.com, mauro.chehab@...wei.com,
        Mauro Carvalho Chehab <mchehab+huawei@...nel.org>,
        Hans Verkuil <hverkuil-cisco@...all.nl>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        linux-kernel@...r.kernel.org, linux-media@...r.kernel.org
Subject: [PATCH 08/11] media: adv7842: better document EDID block size

While the logic there is correct, it leads to smatch warnings:
	/home/hans/work/build/media-git/drivers/media/i2c/adv7842.c:2538 adv7842_set_edid() error: memcpy() '&state->vga_edid.edid' too small (128 vs 512)

Because the code tricks static analyzers by doing:
	memcpy(&state->hdmi_edid.edid, e->edid, 128 * e->blocks);

for ADV7842_EDID_PORT_VGA, where a logic before that makes
e->blocks being either 0 or 1.

Yet, it is ugly to see the "128" magic number all spread about the
EDID code. So, while here, replace 128 (and 4 x 128) by macros:

And ensure that the logic which copy into the VGA block
will use EDID_MAX_VGA_BLOCKS.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
---
 drivers/media/i2c/adv7842.c | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c
index 78e61fe6f2f0..30bddab320b9 100644
--- a/drivers/media/i2c/adv7842.c
+++ b/drivers/media/i2c/adv7842.c
@@ -85,6 +85,10 @@ struct adv7842_format_info {
 	u8 op_format_sel;
 };
 
+#define EDID_BLOCK_SIZE		128
+#define EDID_MAX_HDMI_BLOCKS	4
+#define EDID_MAX_VGA_BLOCKS	1
+
 struct adv7842_state {
 	struct adv7842_platform_data pdata;
 	struct v4l2_subdev sd;
@@ -98,12 +102,12 @@ struct adv7842_state {
 
 	v4l2_std_id norm;
 	struct {
-		u8 edid[512];
+		u8 edid[EDID_BLOCK_SIZE * EDID_MAX_HDMI_BLOCKS];
 		u32 blocks;
 		u32 present;
 	} hdmi_edid;
 	struct {
-		u8 edid[128];
+		u8 edid[EDID_MAX_VGA_BLOCKS * EDID_MAX_VGA_BLOCKS];
 		u32 blocks;
 		u32 present;
 	} vga_edid;
@@ -732,12 +736,13 @@ static int edid_write_vga_segment(struct v4l2_subdev *sd)
 	/* edid segment pointer '1' for VGA port */
 	rep_write_and_or(sd, 0x77, 0xef, 0x10);
 
-	for (i = 0; !err && i < blocks * 128; i += I2C_SMBUS_BLOCK_MAX)
+	for (i = 0; && i < blocks * EDID_BLOCK_SIZE; i += I2C_SMBUS_BLOCK_MAX) {
 		err = i2c_smbus_write_i2c_block_data(state->i2c_edid, i,
 						     I2C_SMBUS_BLOCK_MAX,
 						     edid + i);
-	if (err)
-		return err;
+		if (err)
+			return err;
+	}
 
 	/* Calculates the checksums and enables I2C access
 	 * to internal EDID ram from VGA DDC port.
@@ -785,7 +790,7 @@ static int edid_write_hdmi_segment(struct v4l2_subdev *sd, u8 port)
 		return 0;
 	}
 
-	pa = v4l2_get_edid_phys_addr(edid, blocks * 128, &spa_loc);
+	pa = v4l2_get_edid_phys_addr(edid, blocks * EDID_BLOCK_SIZE, &spa_loc);
 	err = v4l2_phys_addr_validate(pa, &parent_pa, NULL);
 	if (err)
 		return err;
@@ -800,7 +805,7 @@ static int edid_write_hdmi_segment(struct v4l2_subdev *sd, u8 port)
 	}
 
 
-	for (i = 0; !err && i < blocks * 128; i += I2C_SMBUS_BLOCK_MAX) {
+	for (i = 0; !err && i < blocks * EDID_BLOCK_SIZE; i += I2C_SMBUS_BLOCK_MAX) {
 		/* set edid segment pointer for HDMI ports */
 		if (i % 256 == 0)
 			rep_write_and_or(sd, 0x77, 0xef, i >= 256 ? 0x10 : 0x00);
@@ -2491,7 +2496,9 @@ static int adv7842_get_edid(struct v4l2_subdev *sd, struct v4l2_edid *edid)
 	if (edid->start_block + edid->blocks > blocks)
 		edid->blocks = blocks - edid->start_block;
 
-	memcpy(edid->edid, data + edid->start_block * 128, edid->blocks * 128);
+	memcpy(edid->edid,
+	       data + edid->start_block * EDID_BLOCK_SIZE,
+	       edid->blocks * EDID_BLOCK_SIZE);
 
 	return 0;
 }
@@ -2506,9 +2513,12 @@ static int adv7842_get_edid(struct v4l2_subdev *sd, struct v4l2_edid *edid)
 static int adv7842_set_edid(struct v4l2_subdev *sd, struct v4l2_edid *e)
 {
 	struct adv7842_state *state = to_state(sd);
-	unsigned int max_blocks = e->pad == ADV7842_EDID_PORT_VGA ? 1 : 4;
+	unsigned int max_blocks;
 	int err = 0;
 
+	max_blocks = e->pad == ADV7842_EDID_PORT_VGA ?
+		     EDID_MAX_VGA_BLOCKS  : EDID_MAX_HDMI_BLOCKS;
+
 	memset(e->reserved, 0, sizeof(e->reserved));
 
 	if (e->pad > ADV7842_EDID_PORT_VGA)
@@ -2535,7 +2545,7 @@ static int adv7842_set_edid(struct v4l2_subdev *sd, struct v4l2_edid *e)
 		state->vga_edid.blocks = e->blocks;
 		state->vga_edid.present = e->blocks ? 0x1 : 0x0;
 		if (e->blocks)
-			memcpy(&state->vga_edid.edid, e->edid, 128 * e->blocks);
+			memcpy(&state->vga_edid.edid, e->edid, EDID_BLOCK_SIZE);
 		err = edid_write_vga_segment(sd);
 		break;
 	case ADV7842_EDID_PORT_A:
@@ -2544,7 +2554,8 @@ static int adv7842_set_edid(struct v4l2_subdev *sd, struct v4l2_edid *e)
 		state->hdmi_edid.blocks = e->blocks;
 		if (e->blocks) {
 			state->hdmi_edid.present |= 0x04 << e->pad;
-			memcpy(&state->hdmi_edid.edid, e->edid, 128 * e->blocks);
+			memcpy(&state->hdmi_edid.edid, e->edid,
+			       EDID_BLOCK_SIZE * e->blocks);
 		} else {
 			state->hdmi_edid.present &= ~(0x04 << e->pad);
 			adv7842_s_detect_tx_5v_ctrl(sd);
-- 
2.31.1