[vlc-devel] [PATCH] core: use config count not size

[Rémi Denis-Courmont]

Le maanantaina 28. syyskuuta 2020, 20.18.56 EEST Lyndon Brown a écrit :
> On Mon, 2020-09-28 at 18:24 +0300, Rémi Denis-Courmont wrote:
> > OK but IMO this deserves an assertion in the loop that we don't
> > overflow.
> 
> Agreed.
> 
> I felt it best to do so in it's own commit. I also revised the other
> patch to fix the indentation which I noticed was off when adding the
> assertion. Both are attached.
> 
> Preview of the second patch:
> 
> From: Lyndon Brown <jnqnfe at gmail.com>
> Date: Mon, 28 Sep 2020 18:01:37 +0100
> Subject: core: an assert for overflow in config_SortConfig
> 
> 
> diff --git a/src/config/core.c b/src/config/core.c
> index 5016793074..c55096332d 100644
> --- a/src/config/core.c
> +++ b/src/config/core.c
> @@ -33,6 +33,7 @@
> 
>  #include <errno.h>
>  #include <assert.h>
> +#include <limits.h>
> 
>  #include "configuration.h"
>  #include "modules/modules.h"
> @@ -403,7 +404,10 @@ int config_SortConfig (void)
>      size_t nconf = 0;
> 
>      for (p = vlc_plugins; p != NULL; p = p->next)
> +    {
> +        assert((SIZE_MAX - nconf) >= p->conf.count);
>          nconf += p->conf.count;

That's not what I'm after. Integer overflow is not physically possible there.

I mean overflowing the allocated buffer because your patch introduces a non-
trivial assumption about sizes.

> +    }
> 
>      module_config_t **clist = vlc_alloc (nconf, sizeof (*clist));
>      if (unlikely(clist == NULL))


-- 
Реми Дёни-Курмон
http://www.remlab.net/