All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] libsepol: use checked arithmetic builtin to perform safe addition
@ 2021-04-22  6:46 Nicolas Iooss
  2021-04-29 20:24 ` James Carter
  0 siblings, 1 reply; 3+ messages in thread
From: Nicolas Iooss @ 2021-04-22  6:46 UTC (permalink / raw)
  To: selinux

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(-)

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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] libsepol: use checked arithmetic builtin to perform safe addition
  2021-04-22  6:46 [PATCH] libsepol: use checked arithmetic builtin to perform safe addition Nicolas Iooss
@ 2021-04-29 20:24 ` James Carter
  2021-04-30 19:24   ` Nicolas Iooss
  0 siblings, 1 reply; 3+ messages in thread
From: James Carter @ 2021-04-29 20:24 UTC (permalink / raw)
  To: Nicolas Iooss; +Cc: SElinux list

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
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] libsepol: use checked arithmetic builtin to perform safe addition
  2021-04-29 20:24 ` James Carter
@ 2021-04-30 19:24   ` Nicolas Iooss
  0 siblings, 0 replies; 3+ messages in thread
From: Nicolas Iooss @ 2021-04-30 19:24 UTC (permalink / raw)
  To: James Carter; +Cc: SElinux list

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
> >


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-04-30 19:24 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-22  6:46 [PATCH] libsepol: use checked arithmetic builtin to perform safe addition Nicolas Iooss
2021-04-29 20:24 ` James Carter
2021-04-30 19:24   ` Nicolas Iooss

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.