ext/sqlite3: fix wrong pointer types passed to the free list comparator.#21483
ext/sqlite3: fix wrong pointer types passed to the free list comparator.#21483devnexen wants to merge 2 commits intophp:PHP-8.5from
Conversation
SQLite3Stmt::close() and SQLite3Result::finalize() passed php_sqlite3_stmt pointer types instead of sqlite3_stmt pointers to zend_llist_del_element, causing the comparator to never match and both methods to silently become no-ops. Regression introduced in 5eae6d1 ("Don't store the object zval directly").
ndossche
left a comment
ndossche
left a comment
There was a problem hiding this comment.
Are you sure?
https://github.com/devnexen/php-src/blob/18d287e5b50301dd36171e4f3fb0760b23a95021/ext/sqlite3/sqlite3.c#L2265
shows that ->stmt is dereferenced there, and the initialization shows:
https://github.com/devnexen/php-src/blob/18d287e5b50301dd36171e4f3fb0760b23a95021/ext/sqlite3/sqlite3.c#L2417
i.e. elements of type php_sqlite3_stmt * should be added.
|
The pb resides in the second argument not the first static int php_sqlite3_compare_stmt_free(php_sqlite3_stmt **stmt_obj_ptr, sqlite3_stmt *statement ) |
|
by the way here an unchanged line which does not need fix. |
ndossche
left a comment
ndossche
left a comment
There was a problem hiding this comment.
Okay but then the patch here is incomplete as well.
The signature of php_sqlite3_compare_stmt_free is wrong then, as well as its contents. Probably other calls to the linked list code should also be adapted.
In fact, looking at devnexen@bc74cff this code might've always been wrong?
The comparator and all call sites now use php_sqlite3_stmt pointers, matching the type stored in the free list.
ndossche
left a comment
ndossche
left a comment
There was a problem hiding this comment.
I believe this is right, but would like a second opinion too.
|
I also believe this fixes a real long-standing bug that should be backported at a later point after this one is merged in 8.5. |
2 participants
SQLite3Stmt::close() and SQLite3Result::finalize() passed php_sqlite3_stmt pointer types instead of sqlite3_stmt pointers to zend_llist_del_element, causing the comparator to never match and both methods to silently become no-ops.
Regression introduced in 5eae6d1 ("Don't store the object zval directly").