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 /src | |
parent | f61e907d2bbc97160f2d265a7ec6a3ad82b4e6f5 (diff) | |
download | liberasurecode-fix-realloc-bm.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)
Diffstat (limited to 'src')
-rw-r--r-- | src/erasurecode.c | 14 | ||||
-rw-r--r-- | src/erasurecode_helpers.c | 1 | ||||
-rw-r--r-- | src/erasurecode_preprocessing.c | 15 |
3 files changed, 17 insertions, 13 deletions
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; } |