From 4f718a29fe4908c2cea782f751e9805319684e2b Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Mon, 28 Nov 2011 09:44:14 +0100 Subject: firmware: Sigma: Prevent out of bounds memory access The SigmaDSP firmware loader currently does not perform enough boundary size checks when processing the firmware. As a result it is possible that a malformed firmware can cause an out of bounds memory access. This patch adds checks which ensure that both the action header and the payload are completely inside the firmware data boundaries before processing them. Signed-off-by: Lars-Peter Clausen Acked-by: Mike Frysinger Signed-off-by: Mark Brown Cc: stable@kernel.org --- drivers/firmware/sigma.c | 76 +++++++++++++++++++++++++++++++++++------------- 1 file changed, 55 insertions(+), 21 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/sigma.c b/drivers/firmware/sigma.c index f10fc521951b..c780baa59ed9 100644 --- a/drivers/firmware/sigma.c +++ b/drivers/firmware/sigma.c @@ -14,13 +14,34 @@ #include #include -/* Return: 0==OK, <0==error, =1 ==no more actions */ +static size_t sigma_action_size(struct sigma_action *sa) +{ + size_t payload = 0; + + switch (sa->instr) { + case SIGMA_ACTION_WRITEXBYTES: + case SIGMA_ACTION_WRITESINGLE: + case SIGMA_ACTION_WRITESAFELOAD: + payload = sigma_action_len(sa); + break; + default: + break; + } + + payload = ALIGN(payload, 2); + + return payload + sizeof(struct sigma_action); +} + +/* + * Returns a negative error value in case of an error, 0 if processing of + * the firmware should be stopped after this action, 1 otherwise. + */ static int -process_sigma_action(struct i2c_client *client, struct sigma_firmware *ssfw) +process_sigma_action(struct i2c_client *client, struct sigma_action *sa) { - struct sigma_action *sa = (void *)(ssfw->fw->data + ssfw->pos); size_t len = sigma_action_len(sa); - int ret = 0; + int ret; pr_debug("%s: instr:%i addr:%#x len:%zu\n", __func__, sa->instr, sa->addr, len); @@ -29,44 +50,50 @@ process_sigma_action(struct i2c_client *client, struct sigma_firmware *ssfw) case SIGMA_ACTION_WRITEXBYTES: case SIGMA_ACTION_WRITESINGLE: case SIGMA_ACTION_WRITESAFELOAD: - if (ssfw->fw->size < ssfw->pos + len) - return -EINVAL; ret = i2c_master_send(client, (void *)&sa->addr, len); if (ret < 0) return -EINVAL; break; - case SIGMA_ACTION_DELAY: - ret = 0; udelay(len); len = 0; break; - case SIGMA_ACTION_END: - return 1; - + return 0; default: return -EINVAL; } - /* when arrive here ret=0 or sent data */ - ssfw->pos += sigma_action_size(sa, len); - return ssfw->pos == ssfw->fw->size; + return 1; } static int process_sigma_actions(struct i2c_client *client, struct sigma_firmware *ssfw) { - pr_debug("%s: processing %p\n", __func__, ssfw); + struct sigma_action *sa; + size_t size; + int ret; + + while (ssfw->pos + sizeof(*sa) <= ssfw->fw->size) { + sa = (struct sigma_action *)(ssfw->fw->data + ssfw->pos); + + size = sigma_action_size(sa); + ssfw->pos += size; + if (ssfw->pos > ssfw->fw->size || size == 0) + break; + + ret = process_sigma_action(client, sa); - while (1) { - int ret = process_sigma_action(client, ssfw); pr_debug("%s: action returned %i\n", __func__, ret); - if (ret == 1) - return 0; - else if (ret) + + if (ret <= 0) return ret; } + + if (ssfw->pos != ssfw->fw->size) + return -EINVAL; + + return 0; } int process_sigma_firmware(struct i2c_client *client, const char *name) @@ -89,7 +116,14 @@ int process_sigma_firmware(struct i2c_client *client, const char *name) /* then verify the header */ ret = -EINVAL; - if (fw->size < sizeof(*ssfw_head)) + + /* + * Reject too small or unreasonable large files. The upper limit has been + * chosen a bit arbitrarily, but it should be enough for all practical + * purposes and having the limit makes it easier to avoid integer + * overflows later in the loading process. + */ + if (fw->size < sizeof(*ssfw_head) || fw->size >= 0x4000000) goto done; ssfw_head = (void *)fw->data; -- cgit v1.2.2 From c56935bdc0a8edf50237d3b0205133a5b0adc604 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Mon, 28 Nov 2011 09:44:15 +0100 Subject: firmware: Sigma: Skip header during CRC generation The firmware header is not part of the CRC, so skip it. Otherwise the firmware will be rejected due to non-matching CRCs. Signed-off-by: Lars-Peter Clausen Acked-by: Mike Frysinger Signed-off-by: Mark Brown Cc: stable@kernel.org --- drivers/firmware/sigma.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/sigma.c b/drivers/firmware/sigma.c index c780baa59ed9..36265de0a9e8 100644 --- a/drivers/firmware/sigma.c +++ b/drivers/firmware/sigma.c @@ -130,7 +130,8 @@ int process_sigma_firmware(struct i2c_client *client, const char *name) if (memcmp(ssfw_head->magic, SIGMA_MAGIC, ARRAY_SIZE(ssfw_head->magic))) goto done; - crc = crc32(0, fw->data, fw->size); + crc = crc32(0, fw->data + sizeof(*ssfw_head), + fw->size - sizeof(*ssfw_head)); pr_debug("%s: crc=%x\n", __func__, crc); if (crc != ssfw_head->crc) goto done; -- cgit v1.2.2 From bda63586bc5929e97288cdb371bb6456504867ed Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Mon, 28 Nov 2011 09:44:16 +0100 Subject: firmware: Sigma: Fix endianess issues Currently the SigmaDSP firmware loader only works correctly on little-endian systems. Fix this by using the proper endianess conversion functions. Signed-off-by: Lars-Peter Clausen Acked-by: Mike Frysinger Signed-off-by: Mark Brown Cc: stable@kernel.org --- drivers/firmware/sigma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/sigma.c b/drivers/firmware/sigma.c index 36265de0a9e8..1eedb6f7fdab 100644 --- a/drivers/firmware/sigma.c +++ b/drivers/firmware/sigma.c @@ -133,7 +133,7 @@ int process_sigma_firmware(struct i2c_client *client, const char *name) crc = crc32(0, fw->data + sizeof(*ssfw_head), fw->size - sizeof(*ssfw_head)); pr_debug("%s: crc=%x\n", __func__, crc); - if (crc != ssfw_head->crc) + if (crc != le32_to_cpu(ssfw_head->crc)) goto done; ssfw.pos = sizeof(*ssfw_head); -- cgit v1.2.2