| Message ID | 20210422064604.270374-1-nicolas.iooss@m4x.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | libsepol: use checked arithmetic builtin to perform safe addition | expand |
On Thu, Apr 22, 2021 at 2:46 AM Nicolas Iooss <nicolas.iooss@m4x.org> wrote: > > Checking whether an overflow occurred after adding two values can be > achieved using checked arithmetic builtin functions such as: > > bool __builtin_add_overflow(type1 x, type2 y, type3 *sum); > > This function is available at least in clang > (at least since clang 3.8.0, > https://releases.llvm.org/3.8.0/tools/clang/docs/LanguageExtensions.html#checked-arithmetic-builtins) > and gcc > (https://gcc.gnu.org/onlinedocs/gcc/Integer-Overflow-Builtins.html, > since gcc 5 according to https://gcc.gnu.org/gcc-5/changes.html) > > Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org> Acked-by: James Carter <jwcart2@gmail.com> > --- > libsepol/src/context_record.c | 29 ++++++----------------------- > libsepol/src/module_to_cil.c | 6 ++---- > 2 files changed, 8 insertions(+), 27 deletions(-) > > diff --git a/libsepol/src/context_record.c b/libsepol/src/context_record.c > index 317a42133884..435f788058c4 100644 > --- a/libsepol/src/context_record.c > +++ b/libsepol/src/context_record.c > @@ -267,31 +267,13 @@ int sepol_context_from_string(sepol_handle_t * handle, > return STATUS_ERR; > } > > - > -static inline int safe_sum(size_t *sum, const size_t augends[], const size_t cnt) { > - > - size_t a, i; > - > - *sum = 0; > - for(i=0; i < cnt; i++) { > - /* sum should not be smaller than the addend */ > - a = augends[i]; > - *sum += a; > - if (*sum < a) { > - return i; > - } > - } > - > - return 0; > -} > - > int sepol_context_to_string(sepol_handle_t * handle, > const sepol_context_t * con, char **str_ptr) > { > > int rc; > char *str = NULL; > - size_t total_sz, err; > + size_t total_sz = 0, i; > const size_t sizes[] = { > strlen(con->user), /* user length */ > strlen(con->role), /* role length */ > @@ -300,10 +282,11 @@ int sepol_context_to_string(sepol_handle_t * handle, > ((con->mls) ? 3 : 2) + 1 /* mls has extra ":" also null byte */ > }; > > - err = safe_sum(&total_sz, sizes, ARRAY_SIZE(sizes)); > - if (err) { > - ERR(handle, "invalid size, overflow at position: %zu", err); > - goto err; > + for (i = 0; i < ARRAY_SIZE(sizes); i++) { > + if (__builtin_add_overflow(total_sz, sizes[i], &total_sz)) { > + ERR(handle, "invalid size, overflow at position: %zu", i); > + goto err; > + } > } > > str = (char *)malloc(total_sz); > diff --git a/libsepol/src/module_to_cil.c b/libsepol/src/module_to_cil.c > index 58df0d4f6d77..496693f4616e 100644 > --- a/libsepol/src/module_to_cil.c > +++ b/libsepol/src/module_to_cil.c > @@ -1134,16 +1134,14 @@ static int name_list_to_string(char **names, unsigned int num_names, char **stri > char *strpos; > > for (i = 0; i < num_names; i++) { > - len += strlen(names[i]); > - if (len < strlen(names[i])) { > + if (__builtin_add_overflow(len, strlen(names[i]), &len)) { > log_err("Overflow"); > return -1; > } > } > > // add spaces + null terminator > - len += num_names; > - if (len < (size_t)num_names) { > + if (__builtin_add_overflow(len, (size_t)num_names, &len)) { > log_err("Overflow"); > return -1; > } > -- > 2.31.0 >
On Thu, Apr 29, 2021 at 10:24 PM James Carter <jwcart2@gmail.com> wrote: > > On Thu, Apr 22, 2021 at 2:46 AM Nicolas Iooss <nicolas.iooss@m4x.org> wrote: > > > > Checking whether an overflow occurred after adding two values can be > > achieved using checked arithmetic builtin functions such as: > > > > bool __builtin_add_overflow(type1 x, type2 y, type3 *sum); > > > > This function is available at least in clang > > (at least since clang 3.8.0, > > https://releases.llvm.org/3.8.0/tools/clang/docs/LanguageExtensions.html#checked-arithmetic-builtins) > > and gcc > > (https://gcc.gnu.org/onlinedocs/gcc/Integer-Overflow-Builtins.html, > > since gcc 5 according to https://gcc.gnu.org/gcc-5/changes.html) > > > > Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org> > > Acked-by: James Carter <jwcart2@gmail.com> Applied. Thanks, Nicolas > > --- > > libsepol/src/context_record.c | 29 ++++++----------------------- > > libsepol/src/module_to_cil.c | 6 ++---- > > 2 files changed, 8 insertions(+), 27 deletions(-) > > > > diff --git a/libsepol/src/context_record.c b/libsepol/src/context_record.c > > index 317a42133884..435f788058c4 100644 > > --- a/libsepol/src/context_record.c > > +++ b/libsepol/src/context_record.c > > @@ -267,31 +267,13 @@ int sepol_context_from_string(sepol_handle_t * handle, > > return STATUS_ERR; > > } > > > > - > > -static inline int safe_sum(size_t *sum, const size_t augends[], const size_t cnt) { > > - > > - size_t a, i; > > - > > - *sum = 0; > > - for(i=0; i < cnt; i++) { > > - /* sum should not be smaller than the addend */ > > - a = augends[i]; > > - *sum += a; > > - if (*sum < a) { > > - return i; > > - } > > - } > > - > > - return 0; > > -} > > - > > int sepol_context_to_string(sepol_handle_t * handle, > > const sepol_context_t * con, char **str_ptr) > > { > > > > int rc; > > char *str = NULL; > > - size_t total_sz, err; > > + size_t total_sz = 0, i; > > const size_t sizes[] = { > > strlen(con->user), /* user length */ > > strlen(con->role), /* role length */ > > @@ -300,10 +282,11 @@ int sepol_context_to_string(sepol_handle_t * handle, > > ((con->mls) ? 3 : 2) + 1 /* mls has extra ":" also null byte */ > > }; > > > > - err = safe_sum(&total_sz, sizes, ARRAY_SIZE(sizes)); > > - if (err) { > > - ERR(handle, "invalid size, overflow at position: %zu", err); > > - goto err; > > + for (i = 0; i < ARRAY_SIZE(sizes); i++) { > > + if (__builtin_add_overflow(total_sz, sizes[i], &total_sz)) { > > + ERR(handle, "invalid size, overflow at position: %zu", i); > > + goto err; > > + } > > } > > > > str = (char *)malloc(total_sz); > > diff --git a/libsepol/src/module_to_cil.c b/libsepol/src/module_to_cil.c > > index 58df0d4f6d77..496693f4616e 100644 > > --- a/libsepol/src/module_to_cil.c > > +++ b/libsepol/src/module_to_cil.c > > @@ -1134,16 +1134,14 @@ static int name_list_to_string(char **names, unsigned int num_names, char **stri > > char *strpos; > > > > for (i = 0; i < num_names; i++) { > > - len += strlen(names[i]); > > - if (len < strlen(names[i])) { > > + if (__builtin_add_overflow(len, strlen(names[i]), &len)) { > > log_err("Overflow"); > > return -1; > > } > > } > > > > // add spaces + null terminator > > - len += num_names; > > - if (len < (size_t)num_names) { > > + if (__builtin_add_overflow(len, (size_t)num_names, &len)) { > > log_err("Overflow"); > > return -1; > > } > > -- > > 2.31.0 > >
diff --git a/libsepol/src/context_record.c b/libsepol/src/context_record.c index 317a42133884..435f788058c4 100644 --- a/libsepol/src/context_record.c +++ b/libsepol/src/context_record.c @@ -267,31 +267,13 @@ int sepol_context_from_string(sepol_handle_t * handle, return STATUS_ERR; } - -static inline int safe_sum(size_t *sum, const size_t augends[], const size_t cnt) { - - size_t a, i; - - *sum = 0; - for(i=0; i < cnt; i++) { - /* sum should not be smaller than the addend */ - a = augends[i]; - *sum += a; - if (*sum < a) { - return i; - } - } - - return 0; -} - int sepol_context_to_string(sepol_handle_t * handle, const sepol_context_t * con, char **str_ptr) { int rc; char *str = NULL; - size_t total_sz, err; + size_t total_sz = 0, i; const size_t sizes[] = { strlen(con->user), /* user length */ strlen(con->role), /* role length */ @@ -300,10 +282,11 @@ int sepol_context_to_string(sepol_handle_t * handle, ((con->mls) ? 3 : 2) + 1 /* mls has extra ":" also null byte */ }; - err = safe_sum(&total_sz, sizes, ARRAY_SIZE(sizes)); - if (err) { - ERR(handle, "invalid size, overflow at position: %zu", err); - goto err; + for (i = 0; i < ARRAY_SIZE(sizes); i++) { + if (__builtin_add_overflow(total_sz, sizes[i], &total_sz)) { + ERR(handle, "invalid size, overflow at position: %zu", i); + goto err; + } } str = (char *)malloc(total_sz); diff --git a/libsepol/src/module_to_cil.c b/libsepol/src/module_to_cil.c index 58df0d4f6d77..496693f4616e 100644 --- a/libsepol/src/module_to_cil.c +++ b/libsepol/src/module_to_cil.c @@ -1134,16 +1134,14 @@ static int name_list_to_string(char **names, unsigned int num_names, char **stri char *strpos; for (i = 0; i < num_names; i++) { - len += strlen(names[i]); - if (len < strlen(names[i])) { + if (__builtin_add_overflow(len, strlen(names[i]), &len)) { log_err("Overflow"); return -1; } } // add spaces + null terminator - len += num_names; - if (len < (size_t)num_names) { + if (__builtin_add_overflow(len, (size_t)num_names, &len)) { log_err("Overflow"); return -1; }
Checking whether an overflow occurred after adding two values can be achieved using checked arithmetic builtin functions such as: bool __builtin_add_overflow(type1 x, type2 y, type3 *sum); This function is available at least in clang (at least since clang 3.8.0, https://releases.llvm.org/3.8.0/tools/clang/docs/LanguageExtensions.html#checked-arithmetic-builtins) and gcc (https://gcc.gnu.org/onlinedocs/gcc/Integer-Overflow-Builtins.html, since gcc 5 according to https://gcc.gnu.org/gcc-5/changes.html) Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org> --- libsepol/src/context_record.c | 29 ++++++----------------------- libsepol/src/module_to_cil.c | 6 ++---- 2 files changed, 8 insertions(+), 27 deletions(-)