[PATCH v2] media: dvb-frontends: si2168: validate firmware record length

[PATCH v2] media: dvb-frontends: si2168: validate firmware record length

[Pengpeng Hou]

The new Si2168 firmware format stores one command in each 17-byte record:
the first byte is the command length and the remaining 16 bytes are the
command data.

The existing check only compares the length with SI2168_ARGLEN, which
protects the destination command array but not the current firmware record.
A length from 17 through SI2168_ARGLEN makes cmd_init() copy from the next
record, or past the firmware blob for the final record.  An empty firmware
file also passes the modulo test before fw->data[0] is read.

Reject empty files and require each new-format command length to fit the
16-byte payload of its record before copying it.

Fixes: 47810b4341ac ("[media] si2168: Bounds check firmware")
Cc: stable@vger.kernel.org
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
Changes since v1:
- Reject empty firmware blobs before reading the first record byte.
- Name the old and new record sizes while retaining the current-record
  payload bound.

Link: https://lore.kernel.org/r/20260705084706.62129-1-pengpeng@iscas.ac.cn

 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.43.0