PetscTrMallocDefault(): possible buffer overrun

[Lisandro Dalcin]

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?

>
> 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