[Mesa-dev] [PATCH] i965/vec4: Fix hypothetical use of uninitialized data in attribute_map[].

[Jordan Justen]

Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

On Tue, Apr 16, 2013 at 1:37 PM, Paul Berry <stereotype441 at gmail.com> wrote:
> Fixes issue identified by Klocwork analysis:
>
>     'attribute_map' array elements might be used uninitialized in this
>     function (vec4_visitor::lower_attributes_to_hw_regs).
>
> The attribute_map array contains the mapping from shader input
> attributes to the hardware registers they are stored in.
> vec4_vs_visitor::setup_attributes() only populates elements of this
> array which, according to core Mesa, are actually used by the shader.
> Therefore, when vec4_visitor::lower_attributes_to_hw_regs() accesses
> the array to lower a register access in the shader, it should in
> principle only access elements of attribute_map that contain valid
> data.  However, if a bug ever caused the driver back-end to access an
> input that was not flagged as used by core Mesa, then
> lower_attributes_to_hw_regs() would access uninitialized memory, which
> could cause illegal instructions to get generated, resulting in a
> possible GPU hang.
>
> This patch makes the situation more robust by using memset() to
> pre-initialize the attribute_map array to zero, so that if such a bug
> ever occurred, lower_attributes_to_hw_regs() would generate a (mostly)
> harmless access to r0.  In addition, it adds assertions to
> lower_attributes_to_hw_regs() so that if we do have such a bug, we're
> likely to discover it quickly.
> ---
>  src/mesa/drivers/dri/i965/brw_vec4.cpp | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> index b0527c7..0afff6f 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> @@ -1197,6 +1197,11 @@ vec4_visitor::lower_attributes_to_hw_regs(const int *attribute_map)
>        if (inst->dst.file == ATTR) {
>          int grf = attribute_map[inst->dst.reg + inst->dst.reg_offset];
>
> +         /* All attributes used in the shader need to have been assigned a
> +          * hardware register by the caller
> +          */
> +         assert(grf != 0);
> +
>          struct brw_reg reg = brw_vec8_grf(grf, 0);
>          reg.type = inst->dst.type;
>          reg.dw1.bits.writemask = inst->dst.writemask;
> @@ -1211,6 +1216,11 @@ vec4_visitor::lower_attributes_to_hw_regs(const int *attribute_map)
>
>          int grf = attribute_map[inst->src[i].reg + inst->src[i].reg_offset];
>
> +         /* All attributes used in the shader need to have been assigned a
> +          * hardware register by the caller
> +          */
> +         assert(grf != 0);
> +
>          struct brw_reg reg = brw_vec8_grf(grf, 0);
>          reg.dw1.bits.swizzle = inst->src[i].swizzle;
>           reg.type = inst->src[i].type;
> @@ -1230,6 +1240,7 @@ vec4_vs_visitor::setup_attributes(int payload_reg)
>  {
>     int nr_attributes;
>     int attribute_map[VERT_ATTRIB_MAX + 1];
> +   memset(attribute_map, 0, sizeof(attribute_map));
>
>     nr_attributes = 0;
>     for (int i = 0; i < VERT_ATTRIB_MAX; i++) {
> --
> 1.8.2.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev