[Mesa-dev] [PATCH] prog_parameter.c ASAN Patch

[Myles C. Maxfield]

Any word on this?

Thanks,
Myles


On Mon, Jun 17, 2013 at 12:09 PM, Myles C. Maxfield <
myles.maxfield at gmail.com> wrote:

> Sure. I was under the impression that |size| couldn't be both greater than
> 4 and a non-multiple of 4, but I've reworked the patch to incorporate this
> and to be a little more straightforward.
>
> Is the only way to replace "ASAN" with "Address Sanitizer" to change the
> subject of this email thread?
>
> Anyway, here's a similar but modified patch:
>
> From: Myles C. Maxfield <mymax at amazon.com>
> Date: Mon, 17 Jun 2013 11:50:05 -0700
> Subject: [PATCH] Appeasing Address Sanitizer
>
> ---
>  src/mesa/program/prog_parameter.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/program/prog_parameter.c
> b/src/mesa/program/prog_parameter.c
> index 95b153e..1d46476 100644
> --- a/src/mesa/program/prog_parameter.c
> +++ b/src/mesa/program/prog_parameter.c
> @@ -155,7 +155,18 @@ _mesa_add_parameter(struct gl_program_parameter_list
> *paramList,
>           p->Size = size;
>           p->DataType = datatype;
>           if (values) {
> -            COPY_4V(paramList->ParameterValues[oldNum + i], values);
> +            if (size >= (i+1)*4) {
> +                COPY_4V(paramList->ParameterValues[oldNum + i], values);
> +            } else {
> +                /* silence asan */
> +                for (j = 0; j < 4; j++) {
> +                    if (i*4+j < size) {
> +                        paramList->ParameterValues[oldNum + i][j] =
> values[i*4+j];
> +                    } else {
> +                        paramList->ParameterValues[oldNum + i][j].f =
> 0.0f;
> +                    }
> +                }
> +            }
>              values += 4;
>              p->Initialized = GL_TRUE;
>           }
> --
> 1.7.12.4 (Apple Git-37)
>
>
> On Mon, Jun 17, 2013 at 8:13 AM, Brian Paul <brianp at vmware.com> wrote:
>
>> On 06/14/2013 05:12 PM, Myles C. Maxfield wrote:
>>
>>> Sorry for the triple post; I received a bounce email the first time and
>>> got sent to the spam folder the second time, so I'm trying a third time.
>>>
>>> Hello, all. I was running Mesa with Address Sanitizer [1] turned on, and
>>> found one place where ASAN pointed out a read-before-initialized problem.
>>> In particular, in _mesa_add_parameter, in prog_parameter.c, |values|
>>> represents an array holding a variable number of values. These values get
>>> copied out of the array 4 at a time with the COPY_4V macro, however, the
>>> array might only contain a single element. In this case, ASAN reports a
>>> read-before-initialize because the last 3 of the 4 elements haven't been
>>> written to yet. I was hoping to contribute a patch that will silence this
>>> problem that ASAN reports. I'm happy to incorporate any feedback anyone has
>>> into this patch.
>>>
>>> Thanks,
>>> Myles C. Maxfield
>>>
>>> [1]https://code.google.com/p/**address-sanitizer/<https://code.google.com/p/address-sanitizer/>
>>>
>>> diff --git a/src/mesa/program/prog_**parameter.c
>>> b/src/mesa/program/prog_**parameter.c
>>> index 2018fa5..63915fb 100644
>>> --- a/src/mesa/program/prog_**parameter.c
>>> +++ b/src/mesa/program/prog_**parameter.c
>>> @@ -158,7 +158,17 @@ _mesa_add_parameter(struct
>>> gl_program_parameter_list *paramList,
>>>            p->DataType = datatype;
>>>            p->Flags = flags;
>>>            if (values) {
>>> -            COPY_4V(paramList->**ParameterValues[oldNum + i], values);
>>> +            if (size & 3) {
>>> +              for (j = 0; j < size; j++) {
>>> +                paramList->ParameterValues[**oldNum + i][j] =
>>> values[j];
>>> +              }
>>> +              /* silence asan */
>>> +              for (j = size; j < 4; j++) {
>>> +                paramList->ParameterValues[**oldNum + i][j].f = 0;
>>> +              }
>>> +            } else {
>>> +              COPY_4V(paramList->**ParameterValues[oldNum + i],
>>> values);
>>> +            }
>>>               values += 4;
>>>               p->Initialized = GL_TRUE;
>>>            }
>>>
>>
>> The value of 'size' can actually be greater than 4 (IIRC, and the
>> function comment are still correct).  For example, for a matrix, size=16.
>>  So the first for-loop should be fixed, just to be safe.
>>
>> In the commit message, let's not use "ASAN" since it's not obvious that
>> it means Address Sanitizer.
>>
>> -Brian
>>
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130619/6d3f29e8/attachment.html>