Re: [PATCH 5/6 v7] libsepol: Add overflow checks to strs_* family of functions

On Mon, Jul 6, 2026 at 10:39 AM James Carter <[email protected]> wrote:
>
> The strs_* family of functions provide convenient functions for
> handling an array of strings which is used in kernel_to_conf and
> kernel_to_cil. It uses the common idiom of "*= 2" to grow the
> array without any checks for overflow. There is also the function
> strs_add_at_index() which can cause multiple iterations of the
> "*= 2" idiom.
>
> Use the __builtin_mul_overflow() function that will do the
> multiplication while checking for overflow.
>
> In addition, the function strs_len_items() adds up the lengths
> of all the strings in the array which can overflow.
>
> Use the __builtin_add_overflow() function that will do the addition
> while checking for overflow.
>
> Signed-off-by: James Carter <[email protected]>

Acked-by: Stephen Smalley <[email protected]>

> ---
> v7: Remove uneccessary check for len > 0 before subtracting 1
>
>  libsepol/src/kernel_to_common.c | 29 ++++++++++++++++++++++-------
>  libsepol/src/kernel_to_conf.c   | 10 +++++++++-
>  2 files changed, 31 insertions(+), 8 deletions(-)
>
> diff --git a/libsepol/src/kernel_to_common.c b/libsepol/src/kernel_to_common.c
> index adb7b661..c7b6d73a 100644
> --- a/libsepol/src/kernel_to_common.c
> +++ b/libsepol/src/kernel_to_common.c
> @@ -117,13 +117,18 @@ int strs_add(struct strs *strs, char *s)
>         if (strs->num + 1 > strs->size) {
>                 char **new;
>                 size_t i = strs->size;
> -               strs->size *= 2;
> -               new = reallocarray(strs->list, strs->size, sizeof(char *));
> +               size_t new_size;
> +               if (__builtin_mul_overflow(strs->size, 2, &new_size)) {
> +                       ERR(NULL, "Overflow");
> +                       return -1;
> +               }
> +               new = reallocarray(strs->list, new_size, sizeof(char *));
>                 if (!new) {
>                         ERR(NULL, "Out of memory");
>                         return -1;
>                 }
>                 strs->list = new;
> +               strs->size = new_size;
>                 memset(&strs->list[i], 0, sizeof(char *) * (strs->size - i));
>         }
>
> @@ -172,15 +177,20 @@ int strs_add_at_index(struct strs *strs, char *s, size_t index)
>         if (index >= strs->size) {
>                 char **new;
>                 size_t i = strs->size;
> -               while (index >= strs->size) {
> -                       strs->size *= 2;
> +               size_t new_size = strs->size;
> +               while (index >= new_size) {
> +                       if (__builtin_mul_overflow(new_size, 2, &new_size)) {
> +                               ERR(NULL, "Overflow");
> +                               return -1;
> +                       }
>                 }
> -               new = reallocarray(strs->list, strs->size, sizeof(char *));
> +               new = reallocarray(strs->list, new_size, sizeof(char *));
>                 if (!new) {
>                         ERR(NULL, "Out of memory");
>                         return -1;
>                 }
>                 strs->list = new;
> +               strs->size = new_size;
>                 memset(&strs->list[i], 0, sizeof(char *) * (strs->size - i));
>         }
>
> @@ -229,7 +239,9 @@ size_t strs_len_items(const struct strs *strs)
>         for (i = 0; i < strs->num; i++) {
>                 if (!strs->list[i])
>                         continue;
> -               len += strlen(strs->list[i]);
> +               if (__builtin_add_overflow(len, strlen(strs->list[i]), &len)) {
> +                       return SIZE_MAX;
> +               }
>         }
>
>         return len;
> @@ -248,7 +260,10 @@ char *strs_to_str(const struct strs *strs)
>         }
>
>         /* strs->num added because either ' ' or '\0' follows each item */
> -       len = strs_len_items(strs) + strs->num;
> +       if (__builtin_add_overflow(strs_len_items(strs), strs->num, &len)) {
> +               ERR(NULL, "Overflow");
> +               goto exit;
> +       }
>         str = malloc(len);
>         if (!str) {
>                 ERR(NULL, "Out of memory");
> diff --git a/libsepol/src/kernel_to_conf.c b/libsepol/src/kernel_to_conf.c
> index 3e6bcb33..9bfe5b38 100644
> --- a/libsepol/src/kernel_to_conf.c
> +++ b/libsepol/src/kernel_to_conf.c
> @@ -1663,7 +1663,15 @@ static char *attr_strs_to_str(struct strs *strs)
>         }
>
>         /* 2*strs->num - 1 because ", " follows all but last attr (followed by '\0') */
> -       len = strs_len_items(strs) + 2 * strs->num - 1;
> +       if (__builtin_mul_overflow(strs->num, 2, &len)) {
> +               ERR(NULL, "Overflow");
> +               goto exit;
> +       }
> +       len -= 1;
> +       if (__builtin_add_overflow(len, strs_len_items(strs), &len)) {
> +               ERR(NULL, "Overflow");
> +               goto exit;
> +       }
>         str = malloc(len);
>         if (!str) {
>                 ERR(NULL, "Out of memory");
> --
> 2.55.0
>