[PATCH v3] media: dvb-frontends: si2168: validate firmware record bounds

Pengpeng Hou posted 1 patch 1 month, 2 weeks ago
drivers/media/dvb-frontends/si2168.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
[PATCH v3] media: dvb-frontends: si2168: validate firmware record bounds
Posted by Pengpeng Hou 1 month, 2 weeks ago
The new Si2168 firmware format stores a command length followed by
16 payload bytes in each 17-byte record. Checking the length only against
SI2168_ARGLEN protects the destination but not the source record. An
empty image also passes the modulo check before data[0] is read.

Reject empty firmware, name both record sizes, and require each command
to fit the current record payload and destination array.

Fixes: 47810b4341ac ("[media] si2168: Bounds check firmware")
Cc: stable@vger.kernel.org

Assisted-by: Codex:gpt-5
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
Changes since v2: https://lore.kernel.org/all/20260715083945.33781-1-pengpeng@iscas.ac.cn/
- rebase the unchanged source fix on current media sources
- add the coding-assistant disclosure required for this revision

The old/new firmware record extents were reviewed statically; no Si2168
hardware or firmware-loading test was performed.

 drivers/media/dvb-frontends/si2168.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c
index 8bc3b6eb1dd3..182c2b671508 100644
--- a/drivers/media/dvb-frontends/si2168.c
+++ b/drivers/media/dvb-frontends/si2168.c
@@ -11,6 +11,9 @@
 
 static const struct dvb_frontend_ops si2168_ops;
 
+#define SI2168_NEW_FIRMWARE_RECORD_SIZE 17
+#define SI2168_OLD_FIRMWARE_RECORD_SIZE 8
+
 static void cmd_init(struct si2168_cmd *cmd, const u8 *buf, int wlen, int rlen)
 {
 	memcpy(cmd->args, buf, wlen);
@@ -459,11 +462,15 @@ static int si2168_init(struct dvb_frontend *fe)
 	dev_info(&client->dev, "downloading firmware from file '%s'\n",
 			dev->firmware_name);
 
-	if ((fw->size % 17 == 0) && (fw->data[0] > 5)) {
+	if (fw->size &&
+	    fw->size % SI2168_NEW_FIRMWARE_RECORD_SIZE == 0 &&
+	    fw->data[0] > 5) {
 		/* firmware is in the new format */
-		for (remaining = fw->size; remaining > 0; remaining -= 17) {
+		for (remaining = fw->size; remaining > 0;
+		     remaining -= SI2168_NEW_FIRMWARE_RECORD_SIZE) {
 			len = fw->data[fw->size - remaining];
-			if (len > SI2168_ARGLEN) {
+			if (len > SI2168_ARGLEN ||
+			    len >= SI2168_NEW_FIRMWARE_RECORD_SIZE) {
 				ret = -EINVAL;
 				break;
 			}
@@ -473,10 +480,13 @@ static int si2168_init(struct dvb_frontend *fe)
 			if (ret)
 				break;
 		}
-	} else if (fw->size % 8 == 0) {
+	} else if (fw->size &&
+		   fw->size % SI2168_OLD_FIRMWARE_RECORD_SIZE == 0) {
 		/* firmware is in the old format */
-		for (remaining = fw->size; remaining > 0; remaining -= 8) {
-			cmd_init(&cmd, &fw->data[fw->size - remaining], 8, 1);
+		for (remaining = fw->size; remaining > 0;
+		     remaining -= SI2168_OLD_FIRMWARE_RECORD_SIZE) {
+			cmd_init(&cmd, &fw->data[fw->size - remaining],
+				 SI2168_OLD_FIRMWARE_RECORD_SIZE, 1);
 			ret = si2168_cmd_execute(client, &cmd);
 			if (ret)
 				break;
-- 
2.50.1 (Apple Git-155)