[Mesa-dev] [PATCH 1/2] Eliminate several cases of multiplication in arguments to calloc

[Matt Turner]

On Wed, Sep 3, 2014 at 4:09 PM, Carl Worth <cworth at cworth.org> wrote:
> In commit 32f2fd1c5d6088692551c80352b7d6fa35b0cd09, several calls to
> _mesa_calloc(x) were replaced with calls to calloc(1, x). This is strictly
> equivalent to what the code was doing previously.
>
> But for cases where "x" involves multiplication, now that we are explicitly
> using the two-argument calloc, we can do one step better and replace:
>
>         calloc(1, A * B);
>
> with:
>
>         calloc(A, B);
>
> The advantage of the latter is that calloc will detect any overflow that would
> have resulted from the multiplication and will fail the allocation, (whereas
> the former would return a small allocation). So this fix can change
> potentially exploitable buffer overruns into segmentation faults.
> ---
>  src/gallium/drivers/freedreno/a2xx/ir-a2xx.c | 2 +-
>  src/gallium/drivers/freedreno/ir3/ir3.c      | 2 +-
>  src/gallium/drivers/r600/r600_asm.c          | 2 +-
>  src/mapi/glapi/gen/gl_gentable.py            | 2 +-
>  src/mesa/drivers/dri/common/utils.c          | 2 +-
>  src/mesa/drivers/dri/i965/brw_state_cache.c  | 4 ++--
>  src/mesa/main/atifragshader.c                | 8 ++++----
>  src/mesa/program/prog_instruction.c          | 2 +-
>  src/mesa/program/prog_optimize.c             | 6 +++---
>  src/mesa/program/prog_parameter.c            | 2 +-
>  src/mesa/vbo/vbo_exec_array.c                | 2 +-
>  11 files changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/src/gallium/drivers/freedreno/a2xx/ir-a2xx.c b/src/gallium/drivers/freedreno/a2xx/ir-a2xx.c
> index 18afba8..cff5a27 100644
> --- a/src/gallium/drivers/freedreno/a2xx/ir-a2xx.c
> +++ b/src/gallium/drivers/freedreno/a2xx/ir-a2xx.c
> @@ -146,7 +146,7 @@ void * ir2_shader_assemble(struct ir2_shader *shader, struct ir2_shader_info *in
>                 goto fail;
>         }
>
> -       ptr = dwords = calloc(1, 4 * info->sizedwords);
> +       ptr = dwords = calloc(4, info->sizedwords);
>
>         /* second pass, emit CF program in pairs: */
>         for (i = 0; i < shader->cfs_count; i += 2) {
> diff --git a/src/gallium/drivers/freedreno/ir3/ir3.c b/src/gallium/drivers/freedreno/ir3/ir3.c
> index ea2a925..3da10fb 100644
> --- a/src/gallium/drivers/freedreno/ir3/ir3.c
> +++ b/src/gallium/drivers/freedreno/ir3/ir3.c
> @@ -554,7 +554,7 @@ void * ir3_assemble(struct ir3 *shader, struct ir3_info *info)
>          */
>         info->sizedwords = 2 * align(shader->instrs_count, 4);
>
> -       ptr = dwords = calloc(1, 4 * info->sizedwords);
> +       ptr = dwords = calloc(4, info->sizedwords);
>
>         for (i = 0; i < shader->instrs_count; i++) {
>                 struct ir3_instruction *instr = shader->instrs[i];
> diff --git a/src/gallium/drivers/r600/r600_asm.c b/src/gallium/drivers/r600/r600_asm.c
> index 4da918c..8aa69b5 100644
> --- a/src/gallium/drivers/r600/r600_asm.c
> +++ b/src/gallium/drivers/r600/r600_asm.c
> @@ -1590,7 +1590,7 @@ int r600_bytecode_build(struct r600_bytecode *bc)
>                 bc->ndw = cf->addr + cf->ndw;
>         }
>         free(bc->bytecode);
> -       bc->bytecode = calloc(1, bc->ndw * 4);
> +       bc->bytecode = calloc(4, bc->ndw);
>         if (bc->bytecode == NULL)
>                 return -ENOMEM;
>         LIST_FOR_EACH_ENTRY(cf, &bc->cf, list) {
> diff --git a/src/mapi/glapi/gen/gl_gentable.py b/src/mapi/glapi/gen/gl_gentable.py
> index 7577b66..ce9af99 100644
> --- a/src/mapi/glapi/gen/gl_gentable.py
> +++ b/src/mapi/glapi/gen/gl_gentable.py
> @@ -113,7 +113,7 @@ __glapi_gentable_set_remaining_noop(struct _glapi_table *disp) {
>
>  struct _glapi_table *
>  _glapi_create_table_from_handle(void *handle, const char *symbol_prefix) {
> -    struct _glapi_table *disp = calloc(1, _glapi_get_dispatch_table_size() * sizeof(_glapi_proc));
> +    struct _glapi_table *disp = calloc(_glapi_get_dispatch_table_size(), sizeof(_glapi_proc));
>      char symboln[512];
>
>      if(!disp)
> diff --git a/src/mesa/drivers/dri/common/utils.c b/src/mesa/drivers/dri/common/utils.c
> index e0b3db8..1f30966 100644
> --- a/src/mesa/drivers/dri/common/utils.c
> +++ b/src/mesa/drivers/dri/common/utils.c
> @@ -238,7 +238,7 @@ driCreateConfigs(mesa_format format,
>     is_srgb = _mesa_get_format_color_encoding(format) == GL_SRGB;
>
>     num_modes = num_depth_stencil_bits * num_db_modes * num_accum_bits * num_msaa_modes;
> -   configs = calloc(1, (num_modes + 1) * sizeof *configs);
> +   configs = calloc((num_modes + 1), sizeof *configs);

I'd drop the extra parentheses.

With that, both are

Reviewed-by: Matt Turner <mattst88 at gmail.com>