PetscTrMallocDefault(): possible buffer overrun

[Barry Smith]

On Sep 16, 2009, at 10:19 AM, Lisandro Dalcin wrote:

> On Wed, Sep 16, 2009 at 9:58 AM, Barry Smith <bsmith at mcs.anl.gov>  
> wrote:
>>
>>  Lisandro,
>>
>>    You are right; some sloppy stuff here. Back when PetscScalar was  
>> a double
>> and a long an int :-) and before PetscCookie existed.
>>
>
> Yep, I understand. Actually, I discovered this issue by accident,
> looking for bugs at the wrong place.
>
>>    Here is what I suggest, change the two cookie locations to be  
>> declared to
>> be PetscCookie (why not use that type since we have it?)
>> then you can use sizeof(union{PetscCookie,PetscScalar}) for the  
>> extra space.
>>
>
> Of course, PetscCookie should be used.
>
>> I think the original motivation for allocating sizeof(PetscScalar)  
>> in the
>> end location was so that allocated chunks were multiplies of  
>> PetscScalar
>> length.
>> I am not sure if that is important now. Maybe you can just use
>> sizeof(PetscCookie) for the extra space?
>>
>
> If the allocated size being a multiple of sizeof(PetscScalar) is not a
> strong requeriment (cannot imagine why it should be?) I would just use
> sizeof(PetscCookie) for the extra space.
>
> As you are the dad of this baby :-) Could you push the fixes?

    Ok, but it may be a couple of days. I am currently at a workshop  
and am likely to do a half-assed job if I do it while distracted.

    Barry
>
>>
>> On Sep 15, 2009, at 9:07 PM, Lisandro Dalcin wrote:
>>
>>> Barry, please consider Linux 64 bits (but not Win64) and a real,
>>> single precision PetscScalar (i.e. C "float"). Then in this scenario
>>> sizeof(long) is 8 and sizeof(PetscScalar) is 4.
>>>
>>> Now, go to PetscTrMallocDefault() and notice the extra
>>> sizeof(PetscScalar) allocated to save the sentinel cookie at the end
>>> of the buffer. Next, let's see the actual line storing the sentinel
>>> value:
>>>
>>> *(unsigned long *)(inew + nsize) = COOKIE_VALUE;
>>>
>>> Perhaps I'm missing something, but this seems to be a buffer  
>>> overrun:
>>> 8 bytes (unsigned long) will be written, but only 4 bytes (float)  
>>> were
>>> allocated... Again this is for the very specific combination of  
>>> Linux
>>> (OS X?) 64 bits and real single precision PetscScalar. Perhaps we
>>> should allocate sizeof(union{long,PetscScalar}) instead of just
>>> sizeof(PetscScalar) ??
>>>
>>> IIUC, you were the author of this stuff. Then, it would be great if
>>> you could review this :-)
>>>
>>> Thanks,
>>>
>>> --
>>> Lisandro Dalcín
>>> ---------------
>>> Centro Internacional de Métodos Computacionales en Ingeniería  
>>> (CIMEC)
>>> Instituto de Desarrollo Tecnológico para la Industria Química  
>>> (INTEC)
>>> Consejo Nacional de Investigaciones Científicas y Técnicas (CONICET)
>>> PTLC - Güemes 3450, (3000) Santa Fe, Argentina
>>> Tel/Fax: +54-(0)342-451.1594
>>
>>
>
>
>
> -- 
> Lisandro Dalcín
> ---------------
> Centro Internacional de Métodos Computacionales en Ingeniería (CIMEC)
> Instituto de Desarrollo Tecnológico para la Industria Química (INTEC)
> Consejo Nacional de Investigaciones Científicas y Técnicas (CONICET)
> PTLC - Güemes 3450, (3000) Santa Fe, Argentina
> Tel/Fax: +54-(0)342-451.1594