[vtk-developers] vtkDataArrayTemplate.cxx writes out of bounds when malloc fails

Burlen Loring burlen.loring at kitware.com
Fri Jun 20 09:04:54 EDT 2008


Hi David,

Maybe I should have gone into more detail in my description of the 
changes. In this class there is a flag to indicate what the "delete 
method" is as a user can pass in an array to use. They can set either to 
use free or delete. So what I have done in the line you reference is use 
realloc if the array has been marked as created with malloc. This is how 
the class behaved before.

Burlen

David Cole wrote:
> Well, for one thing, you can't do this:
> +    newArray = static_cast<T*>(realloc(this->Array, sz*sizeof(T)));
>
> if you eliminate all the use of malloc. realloc calls don't make any 
> sense if you are using C++ new/delete.
>
>
>
> 2008/6/19 Burlen Loring <burlen.loring at kitware.com 
> <mailto:burlen.loring at kitware.com>>:
>
>     Hi all,
>
>     I would like to get some feed back on a change I would like to
>     make to vtkDataArrayTemplate.txx.
>
>     Here is the problem that prompted the changes:
>
>     Currently during inserts, a data array resizes itself if need be.
>     Unfortunately, in the case that it runs out of memory during the
>     resize, it ignores the fact that it didn't get the needed space
>     and writes into memory it doesn't own. For example take a look at
>     how GetWritePointer is ignoring the return value from
>     ResizeAndExtend which, will be 0 to indicate that malloc failed.
>     And further how InsertNextTuple/InsertTuple has no way of knowing
>     if the malloc failed. The insert then takes place into memory not
>     owned by the data array.
>
>     If you are unfortunate enough to use enough ram that malloc fails
>     during a series of inserts, this can cause some very strange crashes!
>
>     Attached is a patch I'd like to submit to cvs. What I have done is
>     replace the malloc/free pairs with new/delete pairs. Failure of
>     new is much easier to deal with (compared to malloc) because it
>     throws a bad_alloc exception, which clearly indicates the nature
>     of the problem, and the end user can decide how to handle that.
>      Additionally the patch makes sure that if new returns 0 rather
>     than throwing an exception(could happen on older compilers), a
>     return value indicating an error occurred gets propagated back up
>     to the caller immediately. For methods that return pointers the
>     error value is 0 for methods that return vtkIdType the error value
>     is -1. There are a few methods (eg SetTuple) that are typed as
>     returning void, which simply return. In the case of new failures
>     during an insert, this prevents the data array from writing into
>     memory it doesn't own.
>
>     Any objections/criticism/comments regarding this change?
>
>     Thanks in advance
>     Burlen
>
>     -- 
>     Burlen Loring
>     Kitware, Inc.
>     R&D Engineer
>     28 Corporate Drive
>     Clifton Park, NY 12065-8662
>     Phone: 518-371-3971 x137
>
>
>     _______________________________________________
>     vtk-developers mailing list
>     vtk-developers at vtk.org <mailto:vtk-developers at vtk.org>
>     http://www.vtk.org/mailman/listinfo/vtk-developers
>
>


-- 
Burlen Loring
Kitware, Inc.
R&D Engineer
28 Corporate Drive
Clifton Park, NY 12065-8662
Phone: 518-371-3971 x137




More information about the vtk-developers mailing list