Skip to content

Commit 040f39e

Browse files
nordic-mik7nordic-mik7
committed
nrf_compress: fix processing of the 'last part'
This commit fixes the processing of the 'last part' of compressed input. Previously, LZMA_FINISH_END was incorectly enforced and it caused issues if decompression output reached 'dicLimit' during this 'last part' processing. Please refer to following discussion for the detailed rationale: #20582 (comment) Signed-off-by: Michal Kozikowski <[email protected]>
1 parent 56c8ab9 commit 040f39e

File tree

3 files changed

+53
-34
lines changed

3 files changed

+53
-34
lines changed

‎subsys/nrf_compress/src/lzma.c

+10-11
Original file line numberDiff line numberDiff line change
@@ -428,11 +428,11 @@ static int lzma_decompress(void *inst, const uint8_t *input, size_t input_size,
428428
}
429429

430430
#ifdef CONFIG_NRF_COMPRESS_LZMA_VERSION_LZMA2
431-
rc = Lzma2Dec_DecodeToDic(&lzma_decoder, MAX_LZMA_DICT_SIZE, input, &chunk_size,
432-
(last_part ? LZMA_FINISH_END : LZMA_FINISH_ANY), &status);
431+
rc = Lzma2Dec_DecodeToDic(&lzma_decoder, MAX_LZMA_DICT_SIZE,
432+
input, &chunk_size, LZMA_FINISH_ANY, &status);
433433
#else
434-
rc = LzmaDec_DecodeToDic(&lzma_decoder, MAX_LZMA_DICT_SIZE, input, &chunk_size,
435-
(last_part ? LZMA_FINISH_END : LZMA_FINISH_ANY), &status);
434+
rc = LzmaDec_DecodeToDic(&lzma_decoder, MAX_LZMA_DICT_SIZE,
435+
input, &chunk_size, LZMA_FINISH_ANY, &status);
436436
#endif
437437

438438
if (rc) {
@@ -442,8 +442,7 @@ static int lzma_decompress(void *inst, const uint8_t *input, size_t input_size,
442442

443443
*offset = chunk_size;
444444

445-
if (last_part && (status == LZMA_STATUS_FINISHED_WITH_MARK ||
446-
status == LZMA_STATUS_MAYBE_FINISHED_WITHOUT_MARK) &&
445+
if (last_part && status == LZMA_STATUS_FINISHED_WITH_MARK &&
447446
*offset < input_size) {
448447
/* If last block, ensure offset matches complete file size */
449448
*offset = input_size;
@@ -539,19 +538,19 @@ static int lzma_decompress(void *inst, const uint8_t *input, size_t input_size,
539538
}
540539

541540
#ifdef CONFIG_NRF_COMPRESS_LZMA_VERSION_LZMA2
542-
rc = Lzma2Dec_DecodeToDic(&lzma_decoder, decoder->dicHandle->dicBufSize, input, &chunk_size,
543-
LZMA_FINISH_ANY, &status);
541+
rc = Lzma2Dec_DecodeToDic(&lzma_decoder, decoder->dicHandle->dicBufSize,
542+
input, &chunk_size, LZMA_FINISH_ANY, &status);
544543
#else
545-
rc = LzmaDec_DecodeToDic(&lzma_decoder, decoder->dicHandle->dicBufSize, input, &chunk_size,
546-
LZMA_FINISH_ANY, &status);
544+
rc = LzmaDec_DecodeToDic(&lzma_decoder, decoder->dicHandle->dicBufSize,
545+
input, &chunk_size, LZMA_FINISH_ANY, &status);
547546
#endif
548547
if (rc) {
549548
return -EINVAL;
550549
}
551550

552551
*offset = chunk_size;
553552

554-
if (last_part && (status == LZMA_STATUS_FINISHED_WITH_MARK) &&
553+
if (last_part && status == LZMA_STATUS_FINISHED_WITH_MARK &&
555554
*offset < input_size) {
556555
/* If last block, ensure offset matches complete file size. */
557556
*offset = input_size;

‎tests/subsys/nrf_compress/decompression/lzma/CMakeLists.txt

+2-2
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ generate_inc_file_for_target(
1919

2020
generate_inc_file_for_target(
2121
app
22-
${ZEPHYR_NRFXLIB_MODULE_DIR}/tests/subsys/nrf_compress/decompression/dummy_data_input_too_large.txt.lzma
23-
${ZEPHYR_BINARY_DIR}/include/generated/dummy_data_input_too_large.inc
22+
${ZEPHYR_NRFXLIB_MODULE_DIR}/tests/subsys/nrf_compress/decompression/dummy_data_input_large.txt.lzma
23+
${ZEPHYR_BINARY_DIR}/include/generated/dummy_data_input_large.inc
2424
)
2525

2626
generate_inc_file_for_target(

‎tests/subsys/nrf_compress/decompression/lzma/src/main.c

+41-21
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,13 @@ const uint8_t dummy_data_output_sha256[] = {
2828
};
2929

3030
/* Input valid lzma2 compressed data whereby the output is larger than the dictionary size */
31-
const uint8_t dummy_data_too_large_input[] = {
32-
#include "dummy_data_input_too_large.inc"
31+
const uint8_t dummy_data_large_input[] = {
32+
#include "dummy_data_input_large.inc"
3333
};
3434

35-
/* File size and sha256 hash of decompressed data for too large an output */
36-
const uint32_t dummy_data_too_large_output_size = 134061;
37-
const uint8_t dummy_data_too_large_output_sha256[] = {
35+
/* File size and sha256 hash of decompressed data for an output larger than dictionary size */
36+
const uint32_t dummy_data_large_output_size = 134061;
37+
const uint8_t dummy_data_large_output_sha256[] = {
3838
0xc0, 0xc4, 0xac, 0xc7, 0xac, 0x69, 0x37, 0x4b,
3939
0x60, 0xb4, 0x87, 0xe9, 0x3d, 0x65, 0xcf, 0xa2,
4040
0x4b, 0x2b, 0xef, 0xd0, 0xb9, 0xbf, 0xf9, 0xc9,
@@ -205,7 +205,6 @@ ZTEST(nrf_compress_decompression, test_valid_data_decompression)
205205
rc = implementation->decompress(inst, &dummy_data_input[pos],
206206
(sizeof(dummy_data_input) - pos), true,
207207
&offset, &output, &output_size);
208-
pos += 1;
209208
} else {
210209
rc = implementation->decompress(inst, &dummy_data_input[pos], rc, false,
211210
&offset, &output, &output_size);
@@ -257,14 +256,17 @@ ZTEST(nrf_compress_decompression, test_valid_data_decompression)
257256
#endif
258257
}
259258

260-
ZTEST(nrf_compress_decompression, test_valid_data_too_large_decompression)
259+
ZTEST(nrf_compress_decompression, test_valid_data_large_decompression)
261260
{
262261
int rc;
263262
uint32_t pos;
264263
uint32_t offset;
265264
uint8_t *output;
266265
uint32_t output_size;
266+
uint32_t total_output_size = 0;
267+
uint8_t output_sha[SHA256_SIZE] = { 0 };
267268
struct nrf_compress_implementation *implementation;
269+
mbedtls_sha256_context ctx;
268270
#if defined(CONFIG_NRF_COMPRESS_EXTERNAL_DICTIONARY)
269271
void *inst = &lzma_inst;
270272

@@ -273,6 +275,10 @@ ZTEST(nrf_compress_decompression, test_valid_data_too_large_decompression)
273275
void *inst = NULL;
274276
#endif
275277

278+
mbedtls_sha256_init(&ctx);
279+
rc = mbedtls_sha256_starts(&ctx, false);
280+
zassert_ok(rc, "Expected mbedtls sha256 start to be successful");
281+
276282
implementation = nrf_compress_implementation_find(NRF_COMPRESS_TYPE_LZMA);
277283

278284
pos = 0;
@@ -283,12 +289,12 @@ ZTEST(nrf_compress_decompression, test_valid_data_too_large_decompression)
283289
rc = implementation->decompress_bytes_needed(inst);
284290
zassert_equal(rc, 2, "Expected to need 2 bytes for LZMA header");
285291

286-
rc = implementation->decompress(inst, &dummy_data_too_large_input[pos], rc, false, &offset,
292+
rc = implementation->decompress(inst, &dummy_data_large_input[pos], rc, false, &offset,
287293
&output, &output_size);
288294
zassert_ok(rc, "Expected header decompress to be successful");
289295
pos += offset;
290296

291-
while (pos < sizeof(dummy_data_too_large_input)) {
297+
while (pos < sizeof(dummy_data_large_input)) {
292298
rc = implementation->decompress_bytes_needed(inst);
293299
zassert_equal(rc, CONFIG_NRF_COMPRESS_CHUNK_SIZE,
294300
"Expected to need chunk size bytes for LZMA data");
@@ -298,26 +304,43 @@ ZTEST(nrf_compress_decompression, test_valid_data_too_large_decompression)
298304
rc = REDUCED_BUFFER_SIZE;
299305
}
300306

301-
if ((pos + rc) > sizeof(dummy_data_too_large_input)) {
302-
rc = implementation->decompress(inst, &dummy_data_too_large_input[pos],
303-
(sizeof(dummy_data_too_large_input) - pos),
307+
if ((pos + rc) > sizeof(dummy_data_large_input)) {
308+
rc = implementation->decompress(inst, &dummy_data_large_input[pos],
309+
(sizeof(dummy_data_large_input) - pos),
304310
true, &offset, &output, &output_size);
305-
pos += 1;
306311
} else {
307-
rc = implementation->decompress(inst, &dummy_data_too_large_input[pos], rc,
312+
rc = implementation->decompress(inst, &dummy_data_large_input[pos], rc,
308313
false, &offset, &output, &output_size);
309314
}
310315

311-
if (rc != -EINVAL) {
312-
zassert_ok(rc, "Expected data decompress to be successful");
316+
zassert_ok(rc, "Expected data decompress to be successful");
317+
318+
total_output_size += output_size;
319+
320+
if (output_size > 0) {
321+
#if defined(CONFIG_NRF_COMPRESS_EXTERNAL_DICTIONARY)
322+
rc = mbedtls_sha256_update(&ctx, local_dictionary, output_size);
323+
#else
324+
rc = mbedtls_sha256_update(&ctx, output, output_size);
325+
#endif
326+
zassert_ok(rc, "Expected hash update to be successful");
313327
}
314328

315329
pos += offset;
316330
}
317331

318332
(void)implementation->deinit(inst);
333+
zassert_ok(rc, "Expected deinit to be successful");
319334

320-
zassert_equal(rc, -EINVAL, "Expected data decompress with too large an output to fail");
335+
zassert_equal(total_output_size, dummy_data_large_output_size,
336+
"Expected decompressed data size to match");
337+
338+
rc = mbedtls_sha256_finish(&ctx, output_sha);
339+
mbedtls_sha256_free(&ctx);
340+
zassert_ok(rc, "Expected mbedtls sha256 finish to be successful");
341+
342+
zassert_mem_equal(output_sha, dummy_data_large_output_sha256, SHA256_SIZE,
343+
"Expected hash to match");
321344

322345
#if defined(CONFIG_NRF_COMPRESS_EXTERNAL_DICTIONARY)
323346
zassert_equal(open_dict_cnt, 1,
@@ -394,7 +417,7 @@ ZTEST(nrf_compress_decompression, test_invalid_data_data)
394417
rc = implementation->decompress_bytes_needed(inst);
395418
zassert_equal(rc, 2, "Expected to need 2 bytes for LZMA header");
396419

397-
rc = implementation->decompress(inst, &dummy_data_too_large_input[pos], rc, false, &offset,
420+
rc = implementation->decompress(inst, &dummy_data_large_input[pos], rc, false, &offset,
398421
&output, &output_size);
399422
zassert_ok(rc, "Expected header decompress to be successful");
400423
pos += offset;
@@ -413,7 +436,6 @@ ZTEST(nrf_compress_decompression, test_invalid_data_data)
413436
rc = implementation->decompress(NULL, &dummy_data_input[pos],
414437
(sizeof(dummy_data_input) - pos), true,
415438
&offset, &output, &output_size);
416-
pos += 1;
417439
} else if (pos >= REDUCED_BUFFER_SIZE) {
418440
/* Read in manipulated bad data */
419441
uint8_t bad_data[REDUCED_BUFFER_SIZE];
@@ -508,7 +530,6 @@ ZTEST(nrf_compress_decompression, test_valid_data_decompression_random_sizes)
508530
rc = implementation->decompress(inst, &dummy_data_input[pos],
509531
(sizeof(dummy_data_input) - pos), true,
510532
&offset, &output, &output_size);
511-
pos += 1;
512533
} else {
513534
rc = implementation->decompress(inst, &dummy_data_input[pos], rc, false,
514535
&offset, &output, &output_size);
@@ -627,7 +648,6 @@ ZTEST(nrf_compress_decompression, test_valid_data_decompression_reset)
627648
rc = implementation->decompress(inst, &dummy_data_input[pos],
628649
(sizeof(dummy_data_input) - pos), true,
629650
&offset, &output, &output_size);
630-
pos += 1;
631651
} else {
632652
rc = implementation->decompress(inst, &dummy_data_input[pos], rc, false,
633653
&offset, &output, &output_size);

0 commit comments

Comments
 (0)