I found what is effectively a memory corruption bug in all the branches. pecalloc() uses
the wrong length to zero out the memory. Patch is attached, although I'm somewhat
concerned about using just (nmemblen) instead of something like safe_address(nmemblen),
but safe_address() is inlined in zend_alloc.c not in the header file.
We should apply this to 5.2/5.3 before the release.
-Andrei
Hi!
pecalloc() uses the wrong length to zero out the memory. Patch is
attached, although I'm somewhat concerned about using just (nmemblen)
instead of something like safe_address(nmemblen), but safe_address() is
inlined in zend_alloc.c not in the header file.
You just did safe_address in _safe_malloc(nmemb, len, 0) which should
have called E_ERROR if nmemb*len overflows, so do you need to do it again?
Leaving a comment about it wouldn't hurt though :)
Stanislav Malyshev, Zend Software Architect
stas@zend.com http://www.zend.com/
(408)253-8829 MSN: stas@zend.com
Stanislav Malyshev wrote:
You just did safe_address in _safe_malloc(nmemb, len, 0) which should
have calledE_ERRORif nmemb*len overflows, so do you need to do it again?
Leaving a comment about it wouldn't hurt though :)
Yeah, you're right, but I didn't write the code. :) Regardless, I think this fix should go
into 5.2/5.3 because calloc() advertises memory clearing as part of its API contract.
-Andrei
5.2 is already released so you can commit anyway.
For 5.3, I'm not sure (well I am), but wait until Johannes and Lukas
are back online please.
Stanislav Malyshev wrote:
You just did safe_address in _safe_malloc(nmemb, len, 0) which should have
calledE_ERRORif nmemb*len overflows, so do you need to do it again?
Leaving a comment about it wouldn't hurt though :)Yeah, you're right, but I didn't write the code. :) Regardless, I think this
fix should go into 5.2/5.3 because calloc() advertises memory clearing as
part of its API contract.-Andrei
--
--
Pierre
I found what is effectively a memory corruption bug in all the branches. pecalloc() uses
the wrong length to zero out the memory. Patch is attached, although I'm somewhat
concerned about using just (nmemblen) instead of something like safe_address(nmemblen),
but safe_address() is inlined in zend_alloc.c not in the header file.We should apply this to 5.2/5.3 before the release.
Agreed.
johanes
committed.
2009/6/27 Johannes Schlüter johannes@php.net:
I found what is effectively a memory corruption bug in all the branches. pecalloc() uses
the wrong length to zero out the memory. Patch is attached, although I'm somewhat
concerned about using just (nmemblen) instead of something like safe_address(nmemblen),
but safe_address() is inlined in zend_alloc.c not in the header file.We should apply this to 5.2/5.3 before the release.
Agreed.
johanes
--
--
Pierre
Pierre Joye wrote:
committed.
I see that it's not in HEAD though, probably because HEAD has never been updated to use
safe-allocation functions? I see no trace of __zend_calloc() there. We should probably
port that from 5.3.
-Andrei
2009/6/28 Andrei Zmievski andrei@gravitonic.com:
Pierre Joye wrote:
committed.
I see that it's not in HEAD though, probably because HEAD has never been
updated to use safe-allocation functions? I see no trace of __zend_calloc()
there. We should probably port that from 5.3.
Yes, noticed that too while committing them. Thanks for having added
them to the TODO :)
Cheers,
Pierre