Skip to content

Commit

Permalink
[Common] Make manifest sizestr parsing more strict
Browse files Browse the repository at this point in the history
Previously it allowed the string to be followed by any garbage without
an error.

Signed-off-by: Michał Kowalczyk <[email protected]>
  • Loading branch information
mkow committed Aug 31, 2021
1 parent fb615c8 commit 00dcde1
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 30 deletions.
2 changes: 1 addition & 1 deletion Documentation/manifest-syntax.rst
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ Graphene loudly fails with "out of PAL memory" error. To run huge workloads,
increase this limit by setting this option to e.g. ``64M`` (this would result in
a total of 128MB used by Graphene for internal metadata). Note that this limit
is included in ``sgx.enclave_size``, so if your enclave size is e.g. 512MB and
you specify ``loader.pal_internal_mem_size = "64MB"``, then your application is
you specify ``loader.pal_internal_mem_size = "64M"``, then your application is
left with 384MB of usable memory.

Stack size
Expand Down
6 changes: 0 additions & 6 deletions LibOS/shim/src/fs/shim_fs_mem.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,6 @@
#include "shim_fs.h"
#include "shim_fs_mem.h"

#define OVERFLOWS(type, val) \
({ \
type __dummy; \
__builtin_add_overflow((val), 0, &__dummy); \
})

static int mem_file_resize(struct shim_mem_file* mem, size_t buf_size) {
char* buf = malloc(buf_size);
if (!buf)
Expand Down
16 changes: 11 additions & 5 deletions common/include/api.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,12 @@ typedef ptrdiff_t ssize_t;
#define SATURATED_P_SUB(ptr_a, b, limit) \
((__typeof__(ptr_a))SATURATED_SUB((uintptr_t)(ptr_a), (uintptr_t)(b), (uintptr_t)(limit)))

#define OVERFLOWS(type, val) \
({ \
type __dummy; \
__builtin_add_overflow((val), 0, &__dummy); \
})

#define IS_POWER_OF_2(x) \
({ \
assert((x) != 0); \
Expand Down Expand Up @@ -336,16 +342,16 @@ int get_norm_path(const char* path, char* buf, size_t* inout_size);
int get_base_name(const char* path, char* buf, size_t* inout_size);

/*!
* \brief Parse a size (number with optional "G"/"M"/"K" suffix) into an unsigned long.
* \brief Parse a size (number with optional "G"/"M"/"K" suffix) into an uint64_t.
*
* \param str A string containing a non-negative number. The string may end with "G"/"g" suffix
* denoting value in GBs, "M"/"m" for MBs, or "K"/"k" for KBs.
* \param[out] out_val Parsed size (in bytes).
*
* By default the number should be decimal, but if it starts with 0x it is parsed as hexadecimal
* and if it otherwise starts with 0, it is parsed as octal. Function returns -1 if string cannot
* be parsed into a size (e.g., suffix is wrong).
* The number should be decimal. Returns -1 if string cannot be parsed into a size
* (e.g., suffix is wrong).
*/
int64_t parse_size_str(const char* str);
int parse_size_str(const char* str, uint64_t* out_val);

/*!
* \brief Check if a key was specified in TOML manifest.
Expand Down
46 changes: 30 additions & 16 deletions common/src/string/atoi.c
Original file line number Diff line number Diff line change
Expand Up @@ -152,20 +152,34 @@ long int atol(const char* str) {
return value;
}

/* Parse a size (number with optional "G"/"M"/"K" suffix) into an unsigned long. Returns -1 if
* cannot parse the size, e.g. if the suffix is wrong. */
int64_t parse_size_str(const char* str) {
char* endptr = NULL;
long size = strtol(str, &endptr, 0);

if (endptr[0] == 'G' || endptr[0] == 'g')
size *= 1024 * 1024 * 1024;
else if (endptr[0] == 'M' || endptr[0] == 'm')
size *= 1024 * 1024;
else if (endptr[0] == 'K' || endptr[0] == 'k')
size *= 1024;
else if (endptr[0] != '\0')
size = -1; /* wrong suffix */

return size;
int parse_size_str(const char* str, uint64_t* out_val) {
const char* endptr = NULL;
unsigned long size;
int ret = str_to_ulong(str, 10, &size, &endptr);
if (ret < 0)
return -1;

unsigned long unit = 1;
if (*endptr == 'G' || *endptr == 'g') {
unit = 1024 * 1024 * 1024;
endptr++;
} else if (*endptr == 'M' || *endptr == 'm') {
unit = 1024 * 1024;
endptr++;
} else if (*endptr == 'K' || *endptr == 'k') {
unit = 1024;
endptr++;
}

if (__builtin_mul_overflow(size, unit, &size))
return -1;

if (*endptr != '\0')
return -1; /* garbage found after the size string */

if (OVERFLOWS(__typeof__(*out_val), size))
return -1;

*out_val = size;
return 0;
}
5 changes: 3 additions & 2 deletions common/src/string/toml_utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -127,12 +127,13 @@ int toml_sizestring_in(const toml_table_t* root, const char* key, uint64_t defau
}
assert(str);

int64_t ret = parse_size_str(str);
uint64_t size;
int ret = parse_size_str(str, &size);
free(str);

if (ret < 0)
return -1;

*retval = (uint64_t)ret;
*retval = size;
return 0;
}

0 comments on commit 00dcde1

Please sign in to comment.