nrf_compress: Fix ARM thumb filter cross-chunk issue

subsys/nrf_compress/lzma/armthumb.c

@@ -1,4 +1,4 @@
 // SPDX-License-Identifier: 0BSD
 
 ///////////////////////////////////////////////////////////////////////////////
 //
@@ -7,17 +7,21 @@
 ///
 //  Authors:    Igor Pavlov
 //              Lasse Collin
+//  With changes by Nordic Semiconductor ASA
 //
 ///////////////////////////////////////////////////////////////////////////////
 
 #include <stdio.h>
 #include "armthumb.h"
 
-void arm_thumb_filter(uint8_t *buf, uint32_t buf_size, uint32_t pos, bool compress)
+void arm_thumb_filter(uint8_t *buf, uint32_t buf_size, uint32_t pos, bool compress,
+		      bool *end_part_match)
 {
 	uint32_t i = 0;
+	uint32_t last_update_address = 0;
 
 	while ((i + 4) <= buf_size) {
+
 		if ((buf[i + 1] & 0xF8) == 0xF0 && (buf[i + 3] & 0xF8) == 0xF8) {
 			uint32_t dest;
 			uint32_t src = (((uint32_t)(buf[i + 1]) & 7) << 19)
@@ -26,6 +30,7 @@
 			| (uint32_t)(buf[i + 2]);
 
 			src <<= 1;
+			last_update_address = i;
 
 			if (compress) {
 				dest = pos + (uint32_t)(i) + 4 + src;
@@ -38,9 +43,19 @@
 			buf[i + 0] = (dest >> 11);
 			buf[i + 3] = 0xF8 | ((dest >> 8) & 0x7);
 			buf[i + 2] = (dest);
+
 			i += 2;
 		}
 
 		i += 2;
 	}
+
+	if (i == (buf_size - 2))
+	{
+		if (i > last_update_address && (buf[i + 1] & 0xF8) == 0xF0) {
+			*end_part_match = true;
+		} else {
+			*end_part_match = false;
+		}
+	}
 }

subsys/nrf_compress/lzma/armthumb.h

@@ -1,4 +1,4 @@
 // SPDX-License-Identifier: 0BSD
 
 ///////////////////////////////////////////////////////////////////////////////
 //
@@ -16,6 +16,7 @@
 #include <stdint.h>
 #include <stdbool.h>
 
-void arm_thumb_filter(uint8_t *buf, uint32_t buf_size, uint32_t pos, bool compress);
+void arm_thumb_filter(uint8_t *buf, uint32_t buf_size, uint32_t pos, bool compress,
+		      bool *end_part_match);
 
 #endif

subsys/nrf_compress/src/arm_thumb.c

@@ -16,12 +16,18 @@ LOG_MODULE_REGISTER(nrf_compress_arm_thumb, CONFIG_NRF_COMPRESS_LOG_LEVEL);
 BUILD_ASSERT((CONFIG_NRF_COMPRESS_CHUNK_SIZE % 4) == 0,
 	     "CONFIG_NRF_COMPRESS_CHUNK_SIZE must be multiple of 4");
 
-static uint8_t output_buffer[CONFIG_NRF_COMPRESS_CHUNK_SIZE];
+/* Requires 2 extra bytes to allow checking cross-chunk 16-bit aligned ARM thumb instructions */
+#define EXTRA_BUFFER_SIZE 2
+
+static uint8_t output_buffer[CONFIG_NRF_COMPRESS_CHUNK_SIZE + EXTRA_BUFFER_SIZE];
+static uint8_t temp_extra_buffer[EXTRA_BUFFER_SIZE];
 static uint32_t data_position = 0;
+static bool has_extra_buffer_data;
 
 static int arm_thumb_init(void *inst)
 {
 	data_position = 0;
+	has_extra_buffer_data = false;
 
 	return 0;
 }
@@ -38,6 +44,7 @@ static int arm_thumb_deinit(void *inst)
 static int arm_thumb_reset(void *inst)
 {
 	data_position = 0;
+	has_extra_buffer_data = false;
 	memset(output_buffer, 0x00, sizeof(output_buffer));
 
 	return 0;
@@ -52,17 +59,47 @@ static int arm_thumb_decompress(void *inst, const uint8_t *input, size_t input_s
 				bool last_part, uint32_t *offset, uint8_t **output,
 				size_t *output_size)
 {
+	bool end_part_match = false;
+	bool extra_buffer_used = false;
+
 	if (input_size > CONFIG_NRF_COMPRESS_CHUNK_SIZE) {
 		return -EINVAL;
 	}
 
-	memcpy(output_buffer, input, input_size);
-	arm_thumb_filter(output_buffer, input_size, data_position, false);
+	if (has_extra_buffer_data == true) {
+		/* Copy bytes from temporary holding buffer */
+		memcpy(output_buffer, temp_extra_buffer, sizeof(temp_extra_buffer));
+		memcpy(&output_buffer[sizeof(temp_extra_buffer)], input, input_size);
+		end_part_match = true;
+		extra_buffer_used = true;
+		has_extra_buffer_data = false;
+		input_size += sizeof(temp_extra_buffer);
+	} else {
+		memcpy(output_buffer, input, input_size);
+	}
+
+	arm_thumb_filter(output_buffer, input_size, data_position, false, &end_part_match);
 	data_position += input_size;
 	*offset = input_size;
+
+	if (extra_buffer_used) {
+		*offset -= sizeof(temp_extra_buffer);
+	}
+
 	*output = output_buffer;
 	*output_size = input_size;
 
+	if (end_part_match == true && !last_part) {
+		/* Partial match at end of input, need to cut the final 2 bytes off and stash
+		 * them
+		 */
+		memcpy(temp_extra_buffer, &output_buffer[(input_size - sizeof(temp_extra_buffer))],
+		       sizeof(temp_extra_buffer));
+		has_extra_buffer_data = true;
+		*output_size -= sizeof(temp_extra_buffer);
+		data_position -= sizeof(temp_extra_buffer);
+	}
+
 	return 0;
 }
 

tests/subsys/nrf_compress/decompression/arm_thumb/src/main.c

@@ -18,13 +18,16 @@ static const uint8_t output_expected[] = {
 #include "arm_thumb.inc"
 };
 
+#define PLUS_MINUS_OUTPUT_SIZE 2
+
 ZTEST(nrf_compress_decompression, test_valid_implementation)
 {
 	int rc;
 	uint32_t pos = 0;
 	uint32_t offset;
 	uint8_t *output;
 	uint32_t output_size;
+	uint32_t total_output_size = 0;
 	struct nrf_compress_implementation *implementation = NULL;
 
 	implementation = nrf_compress_implementation_find(NRF_COMPRESS_TYPE_ARM_THUMB);
@@ -36,26 +39,36 @@ ZTEST(nrf_compress_decompression, test_valid_implementation)
 
 	while (pos < sizeof(input_compressed)) {
 		uint32_t input_data_size;
+		bool last = false;
 
 		input_data_size = implementation->decompress_bytes_needed(NULL);
 		zassert_equal(input_data_size, CONFIG_NRF_COMPRESS_CHUNK_SIZE,
 			"Expected to need chunk size bytes for LZMA data");
 
 		if ((pos + input_data_size) >= sizeof(input_compressed)) {
 			input_data_size = sizeof(input_compressed) - pos;
+			last = true;
 		}
 
 		rc = implementation->decompress(NULL, &input_compressed[pos], input_data_size,
-						false, &offset, &output, &output_size);
+						last, &offset, &output, &output_size);
 
 		zassert_ok(rc, "Expected data decompress to be successful");
-		zassert_equal(output_size, input_data_size,
-			      "Expected data decompress to be successful");
 		zassert_mem_equal(output, &output_expected[pos], output_size);
 
+		if (!(output_size == input_data_size || output_size ==
+		      (input_data_size + PLUS_MINUS_OUTPUT_SIZE) || output_size ==
+		      (input_data_size - PLUS_MINUS_OUTPUT_SIZE))) {
+			zassert_ok(1, "Expected data output size is not valid");
+		}
+
 		pos += offset;
+		total_output_size += output_size;
 	}
 
+	zassert_equal(total_output_size, sizeof(output_expected),
+		      "Expected data decompress output size to match data input size");
+
 	rc = implementation->deinit(NULL);
 	zassert_ok(rc, "Expected deinit to be successful");
 }

west.yml

@@ -132,7 +132,7 @@ manifest:
           compare-by-default: true
     - name: mcuboot
       repo-path: sdk-mcuboot
-      revision: 720fa02787366f9f787b847194f6814921147770
+      revision: pull/353/head
       path: bootloader/mcuboot
     - name: qcbor
       url: https://github.com/laurencelundblade/QCBOR