Fix #80663: Recursive SplFixedArray::setSize() may cause double-free #7485
Conversation
We disallow recursive calls to SplFixedArray::setSize() to avoid that, and throw a LogicException instead.
|
offsetSet and offsetUnset also have potential issues that I filed after seeing this was getting worked on - see https://bugs.php.net/bug.php?id=81429
It's also possible to copy the byte ranges to destroy before shrinking and reallocating the array, separately freeing the byte ranges, but it should be possible to fix even without doing that. I'd went with that approach for https://github.com/TysonAndre/pecl-teds/blob/6ec8dd2037850fa3775f17579ebba86ebd87f35c/teds_vector.c#L454-L472 |
|
Thanks, @TysonAndre! I'll have a closer look at this. |
| @@ -43,6 +43,8 @@ ZEND_GET_MODULE(spl_fixedarray) | |||
| typedef struct _spl_fixedarray { /* {{{ */ | |||
| zend_long size; | |||
| zval *elements; | |||
| unsigned int is_resizing:1; | |||
There was a problem hiding this comment.
Compared to creating a temporary copy of zvals to destroy, this would also add another 4 or 8 bytes (with 64-bit struct field alignment?) to each SplFixedArray instance (and a tiny bit more time initializing it), when most applications wouldn't use setSize (according to other remarks it was meant more as an escape hatch than as a way to support vector-like functionality)
Some progress was made in reducing SplFixedArray's memory usage and instantiation time in #6552
|
Maybe address the |
We disallow recursive calls to SplFixedArray::setSize() to avoid that,
and throw a LogicException instead.
If we are concerned about the BC break (at least some recursive resize operations don't appear to be a real issue), we could ignore the operation and raise E_WARNING instead of throwing.