diff options
author | Kota Tsuyuzaki <bloodeagle40234@gmail.com> | 2015-04-17 00:31:53 -0700 |
---|---|---|
committer | Kota Tsuyuzaki <bloodeagle40234@gmail.com> | 2015-07-15 17:17:03 -0700 |
commit | 453beb542b51e86cc802e372a63df8dc085bb387 (patch) | |
tree | 09ef406cc30b5b65d6ef1cc74f0b319255b4ed8b | |
parent | f61e907d2bbc97160f2d265a7ec6a3ad82b4e6f5 (diff) | |
download | liberasurecode-453beb542b51e86cc802e372a63df8dc085bb387.tar.gz |
Fix decode realloc bitmap segfaultfix-realloc-bm
When using an instance with k + m parameter more than 32
(e.g. k=30, m=20), decoding process might free invalid fragments
memory passed from an upper application and it might cause double
free corruption on upper application layer. That is because some
of realloc_bm calculation like as follows might make invalid handling
to free memory.
e.g. (for reproducing):
- k=30, m=20
- available fragments 0, 1~49 (i.e. fragment 1 dropped) passed in.
and then after decoding...
- liberasurecode frees the realloc memory for fragment 1 *AND 33 (1 + 32)*!
When realloc_bm = 2 (i.e. wants to free only data[1]),
that if-condition results in as folows:
The result of (realloc_bm & 1 << i):
i = 0 -> 0
i = 1 -> 2
i = 2~32 -> 0
i = 33 -> 2 (overflowing!!!)
This overflowing makes liberasurecode to free the memory for fragment
33, though the memory must not be freed.
To prevent this corruption, this patch makes the base integer
as 64bit and liberasurecode to raise an Exception when k + m > 64
at initialization. (i.e. we cannot use more than 64 fragments with
current realloc_bm because it is an instance based on int64)
-rw-r--r-- | include/erasurecode/erasurecode.h | 4 | ||||
-rw-r--r-- | src/erasurecode.c | 14 | ||||
-rw-r--r-- | src/erasurecode_helpers.c | 1 | ||||
-rw-r--r-- | src/erasurecode_preprocessing.c | 15 | ||||
-rw-r--r-- | test/liberasurecode_test.c | 32 |
5 files changed, 53 insertions, 13 deletions
diff --git a/include/erasurecode/erasurecode.h b/include/erasurecode/erasurecode.h index cfceb7b..9b4d129 100644 --- a/include/erasurecode/erasurecode.h +++ b/include/erasurecode/erasurecode.h @@ -32,6 +32,10 @@ #include "list.h" #include "erasurecode_stdinc.h" #include "erasurecode_version.h" +#include <stdint.h> + +#define BASE_64BIT (uint64_t)1 +#define MAX_FRAGMENTS_NUM 64 #ifdef __cplusplus extern "C" { diff --git a/src/erasurecode.c b/src/erasurecode.c index 57054b9..7d8ef7d 100644 --- a/src/erasurecode.c +++ b/src/erasurecode.c @@ -253,6 +253,12 @@ int liberasurecode_instance_create(const ec_backend_id_t id, if (id >= EC_BACKENDS_MAX) return -EBACKENDNOTSUPP; + if ((args->k + args->m) > MAX_FRAGMENTS_NUM){ + log_error("Fragments sum (k + m) must be less than %d\n", + MAX_FRAGMENTS_NUM); + return -EINVALIDPARAMS; + } + /* Allocate memory for ec_backend instance */ instance = calloc(1, sizeof(*instance)); if (NULL == instance) @@ -683,13 +689,13 @@ out: /* Free the buffers allocated in prepare_fragments_for_decode */ if (realloc_bm != 0) { for (i = 0; i < k; i++) { - if (realloc_bm & (1 << i)) { + if (realloc_bm & (BASE_64BIT << i)) { free(data[i]); } } for (i = 0; i < m; i++) { - if (realloc_bm & (1 << (i + k))) { + if (realloc_bm & (BASE_64BIT << (i + k))) { free(parity[i]); } } @@ -873,13 +879,13 @@ out: /* Free the buffers allocated in prepare_fragments_for_decode */ if (realloc_bm != 0) { for (i = 0; i < k; i++) { - if (realloc_bm & (1 << i)) { + if (realloc_bm & (BASE_64BIT << i)) { free(data[i]); } } for (i = 0; i < m; i++) { - if (realloc_bm & (1 << (i + k))) { + if (realloc_bm & (BASE_64BIT << (i + k))) { free(parity[i]); } } diff --git a/src/erasurecode_helpers.c b/src/erasurecode_helpers.c index 522766e..560dc34 100644 --- a/src/erasurecode_helpers.c +++ b/src/erasurecode_helpers.c @@ -123,7 +123,6 @@ char *alloc_fragment_buffer(int size) size += sizeof(fragment_header_t); buf = get_aligned_buffer16(size); - if (buf) { header = (fragment_header_t *) buf; header->magic = LIBERASURECODE_FRAG_HEADER_MAGIC; diff --git a/src/erasurecode_preprocessing.c b/src/erasurecode_preprocessing.c index 06fd1cc..61258b5 100644 --- a/src/erasurecode_preprocessing.c +++ b/src/erasurecode_preprocessing.c @@ -116,11 +116,11 @@ int prepare_fragments_for_decode( uint64_t *realloc_bm) { int i; /* a counter */ - unsigned long long missing_bm; /* bitmap form of missing indexes list */ + uint64_t missing_bm; /* bitmap form of missing indexes list */ int orig_data_size = -1; int payload_size = -1; - missing_bm = convert_list_to_bitmap(missing_idxs); + missing_bm = (uint64_t)convert_list_to_bitmap(missing_idxs); /* * Determine if each data fragment is: @@ -141,7 +141,7 @@ int prepare_fragments_for_decode( log_error("Could not allocate data buffer!"); return -ENOMEM; } - *realloc_bm = *realloc_bm | (1 << i); + *realloc_bm = *realloc_bm | (BASE_64BIT << i); } else if (!is_addr_aligned((unsigned long)data[i], 16)) { char *tmp_buf = alloc_fragment_buffer(fragment_size - sizeof(fragment_header_t)); if (NULL == tmp_buf) { @@ -150,11 +150,11 @@ int prepare_fragments_for_decode( } memcpy(tmp_buf, data[i], fragment_size); data[i] = tmp_buf; - *realloc_bm = *realloc_bm | (1 << i); + *realloc_bm = *realloc_bm | (BASE_64BIT << i); } /* Need to determine the size of the original data */ - if (((missing_bm & (1 << i)) == 0) && orig_data_size < 0) { + if (((missing_bm & (BASE_64BIT << i)) == 0) && orig_data_size < 0) { orig_data_size = get_orig_data_size(data[i]); if (orig_data_size < 0) { log_error("Invalid orig_data_size in fragment header!"); @@ -180,7 +180,7 @@ int prepare_fragments_for_decode( log_error("Could not allocate parity buffer!"); return -ENOMEM; } - *realloc_bm = *realloc_bm | (1 << (k + i)); + *realloc_bm = *realloc_bm | (BASE_64BIT << (k + i)); } else if (!is_addr_aligned((unsigned long)parity[i], 16)) { char *tmp_buf = alloc_fragment_buffer(fragment_size-sizeof(fragment_header_t)); if (NULL == tmp_buf) { @@ -189,7 +189,7 @@ int prepare_fragments_for_decode( } memcpy(tmp_buf, parity[i], fragment_size); parity[i] = tmp_buf; - *realloc_bm = *realloc_bm | (1 << (k + i)); + *realloc_bm = *realloc_bm | (BASE_64BIT << (k + i)); } /* Need to determine the size of the original data */ @@ -210,7 +210,6 @@ int prepare_fragments_for_decode( *orig_size = orig_data_size; *fragment_payload_size = payload_size; - return 0; } diff --git a/test/liberasurecode_test.c b/test/liberasurecode_test.c index ff4641a..77d4b73 100644 --- a/test/liberasurecode_test.c +++ b/test/liberasurecode_test.c @@ -488,6 +488,18 @@ static void test_create_backend_invalid_args() return; } assert(desc < 0); + + struct ec_args invalid_args = { + .k = 100, + .m = 100, + }; + + desc = liberasurecode_instance_create(EC_BACKEND_NULL, &invalid_args); + if (-EBACKENDNOTAVAIL == desc) { + fprintf (stderr, "Backend library not available!\n"); + return; + } + assert(desc < 0); } static void test_destroy_backend_invalid_args() @@ -1316,6 +1328,22 @@ static void test_simple_encode_decode(const ec_backend_id_t be_id, free(skip); } +static void test_simple_encode_decode_over32() +{ + struct ec_args over32_args = { + .k = 30, + .m = 20, + }; + + // k + m > 32 condition must not free any other fragments + // memory except missing fragment 1 + int *skip = create_skips_array(&over32_args, 1); + assert(skip != NULL); + encode_decode_test_impl(EC_BACKEND_JERASURE_RS_VAND, + &over32_args, skip); + free(skip); +} + static void test_simple_reconstruct(const ec_backend_id_t be_id, struct ec_args *args) { @@ -1655,6 +1683,10 @@ struct testcase testcases[] = { test_verify_stripe_metadata_be_ver_mismatch, EC_BACKEND_JERASURE_RS_VAND, CHKSUM_CRC32, .skip = false}, + {"test_simple_encode_decode_over32", + test_simple_encode_decode_over32, + EC_BACKEND_JERASURE_RS_VAND, CHKSUM_CRC32, + .skip = false}, // Jerasure RS Cauchy backend tests {"create_and_destroy_backend", test_create_and_destroy_backend, |