[CRIU] [Patch] Fix UB in choose_service_fd_base due to calling __builtin_clz(0)

Kirill Tkhai ktkhai at virtuozzo.com
Mon Apr 9 19:03:14 MSK 2018


On 09.04.2018 17:57, Radoslaw Burny wrote:
> From: Radoslaw Burny <rburny at google.com>
> 
> Subject: [PATCH] Fix UB in choose_service_fd_base.
> 
> Signed-off-by: Radoslaw Burny <rburny at google.com>
> 
> ---
>  criu/util.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/criu/util.c b/criu/util.c
> index b19bf517..48ba09a8 100644
> --- a/criu/util.c
> +++ b/criu/util.c
> @@ -588,9 +588,9 @@ static int choose_service_fd_base(struct pstree_item
> *me)
>         nr += 16; /* Safety pad */
>         real_nr = nr;
> 
> -       nr /= (1024 / sizeof(void *));
> +       /* Align nr to the power of 2 for easier debugging */
> +       BUG_ON(nr <= 0);
>         nr = 1 << (32 - __builtin_clz(nr));
> -       nr *= (1024 / sizeof(void *));
> 
>         if (nr > service_fd_rlim_cur) {
>                 /* Right border is bigger, than rlim. OK, then just aligned
> value is enough */

It's not a round up power of 2, it's round up power of 2 rounded to 128.
This goes from kernel: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/file.c#n85

#include <limits.h>
int main(void)
{
unsigned int i, was, nr;

    for (i = 0; i < INT_MAX; i++) {
            nr = i;
            nr /= (1024 / sizeof(void *));
            if (nr)
                    nr = 1 << (32 - __builtin_clz(nr));
            else
                    nr = 1;
            nr *= (1024 / sizeof(void *));

            printf("nr=%d, i=%d\n", nr, i);
            if (nr < i || (nr-1) & nr) {
                    printf("error\n");
                    exit(1);
            }
    }

    return 0;

}

nr=128, i=16
nr=128, i=17
nr=128, i=18
nr=128, i=19
nr=128, i=20
nr=128, i=21
nr=128, i=22
nr=128, i=23
nr=128, i=24
nr=128, i=25
nr=128, i=26
nr=128, i=27
nr=128, i=28
nr=128, i=29
nr=128, i=30


More information about the CRIU mailing list