[vlc-devel] [VLC] #2366: realloc is generally used incorrectly
fenrir at via.ecp.fr
Tue May 26 00:34:02 CEST 2009
On Mon, May 25, 2009, Rémi Denis-Courmont wrote:
> Assuming that we cannot:
> * detect out-of-memory (due to demand-paging),
> * detect out-of-address space (due to underlying libraries),
> * do much "good" even when we detect either of those,
> I think crashing the process is just fine, for lack of a better alternative.
> >From the security perspective, a really bad problem occurs if there is a
> "large" write offset into the allocated memory. If we just write at 0x0 or
> close, then a segmentation fault will occur (so long as page zero is not
> mapped, which would imply we were hacked already).
> Indeed NULL + offset = offset = evil things possible.
> (Integer overflow into the allocation size are another issue by the way.)
> > The code path from a malloc failure is in practice never tested, and so
> > will be full of bugs. There is no way to escape that. Limiting the
> > attention to where it is really dangerous (or more probable) will probably
> > make a better code and ease dev and so increase code quality.
> On a high-level, I can only agree. But I don't know how to "decide" this on a
> low-level. Perhaps the simplest yet is to do if (NULL) abort(); wherever we do
I think simply returning from the function without releasing anything that is
just memory will simplify a lot and is easy to be safe. Yes, when not enough
memory (and is detected) we will loose a bit more but better that than a
crash/security risk and might be a good compromise. abort() could be reserved
for cases where some states are corrupted and not recovered
More information about the vlc-devel