ID: 48668
User updated by: dmda at yandex dot ru
Reported By: dmda at yandex dot ru
Status: Critical
Bug Type: Reproducible crash
Operating System: Solaris
PHP Version: 5.3.0RC4
Assigned To: dsp
New Comment:
to demonstrate how it works, I wrote some trivial examples:
1) bad coding practice or misligned memory operations that lead to Bus
Error:
#include "inttypes.h"
typedef struct {
int16_t i;
} s16;
typedef struct {
int32_t i;
} s32;
int main() {
char *c = (char*)malloc(sizeof(s16) + sizeof(s32));
s16* v1 = (void*)(c);
s32* v2 = (void*)((char*)v1 + sizeof(s16));
printf("&v1=%x\n", v1);
printf("&v2=%x\n", v2);
v2->i = 55;
printf("ok\n");
return 0;
}
you'll note that the address of v2 is not aligned to 32bit boundary and
therefore v2->i = 55 will crash:
$gcc a.c -o a
$./a
&v1=208e8
&v2=208ea
Bus Error (core dumped)
2) Fix with MEMORY ALIGNMENT
#include "inttypes.h"
typedef struct {
int16_t i;
} s16;
typedef struct {
int32_t i;
} s32;
#define ALIGNGRANUL 4
#define ALIGN(a) ((a + ALIGNGRANUL - 1) & ~(ALIGNGRANUL - 1))
int main() {
char *c = (char*)malloc(ALIGN(sizeof(s16)) + ALIGN(sizeof(s32)));
s16* v1 = (void*)(c);
s32* v2 = (void*)((char*)v1 + ALIGN(sizeof(s16)));
printf("&v1=%x\n", v1);
printf("&v2=%x\n", v2);
v2->i = 55;
printf("ok\n");
return 0;
}
you'll see that all addresses are properly aligned to 32bit boundary:
$gcc a.c -o a
$./a
&v1=208e8
&v2=208ec
ok
3) a better fix that does not need any knowledge about memory
alignment:
#include "inttypes.h"
typedef struct {
int16_t i;
} s16;
typedef struct {
int32_t i;
} s32;
typedef struct {
s16 v1;
s32 v2;
} ss;
int main() {
ss* v = malloc(sizeof(ss));
printf("&v1=%x\n", &v->v1);
printf("&v2=%x\n", &v->v2);
v->v2.i = 55;
printf("ok\n");
return 0;
}
you'll see that it works too:
$gcc a.c -o a
$./a
&v1=208e8
&v2=208ec
ok
so you see this is all around sizeof() :) while actually it's all about
memory alignment.
Previous Comments:
------------------------------------------------------------------------
[2009-08-18 17:42:20] dmda at yandex dot ru
Dear sriram ,
Where did I say that the problem is in allocation size?
Once again, the problem is in the data alignment.
And fixing the code I mentioned, it fixes the Bus error.
diff -U5 ./php-5.3.0/Zend/zend_execute.h
./php-5.3.0D/Zend/zend_execute.h
--- ./php-5.3.0/Zend/zend_execute.h 2009-06-09 05:26:02.000000000
-0400
+++ ./php-5.3.0D/Zend/zend_execute.h 2009-08-18 12:27:18.080008000
-0400
@@ -154,11 +154,11 @@
zend_vm_stack_extend((count) TSRMLS_CC);
\
}
\
} while (0)
static inline zend_vm_stack zend_vm_stack_new_page(int count) {
- zend_vm_stack page =
(zend_vm_stack)emalloc(sizeof(*page)+sizeof(page->elements[0])*(count-1));
+ zend_vm_stack page =
(zend_vm_stack)emalloc(ZEND_MM_ALIGNED_SIZE(sizeof(*page))+ZEND_MM_ALIGNED_SIZE(sizeof(page->elements[0]))*(count-1));
page->top = page->elements;
page->end = page->elements + count;
page->prev = NULL;
return page;
@@ -217,11 +217,11 @@
static inline void *zend_vm_stack_alloc(size_t size TSRMLS_DC)
{
void *ret;
- size = (size + (sizeof(void*) - 1)) / sizeof(void*);
+ size = (size + (ZEND_MM_ALIGNED_SIZE(sizeof(void*)) - 1)) /
ZEND_MM_ALIGNED_SIZE(sizeof(void*));
ZEND_VM_STACK_GROW_IF_NEEDED((int)size);
ret = (void*)EG(argument_stack)->top;
EG(argument_stack)->top += size;
return ret;
diff -U5 ./php-5.3.0/Zend/zend_opcode.c
./php-5.3.0D/Zend/zend_opcode.c
--- ./php-5.3.0/Zend/zend_opcode.c 2009-06-05 19:20:59.000000000 -0400
+++ ./php-5.3.0D/Zend/zend_opcode.c 2009-08-18 12:29:21.630007000
-0400
@@ -43,11 +43,11 @@
}
}
static void op_array_alloc_ops(zend_op_array *op_array)
{
- op_array->opcodes = erealloc(op_array->opcodes,
(op_array->size)*sizeof(zend_op));
+ op_array->opcodes = erealloc(op_array->opcodes,
(op_array->size)*ZEND_MM_ALIGNED_SIZE(sizeof(zend_op)));
}
void init_op_array(zend_op_array *op_array, zend_uchar type, int
initial_ops_size TSRMLS_DC)
{
op_array->type = type;
@@ -287,11 +287,11 @@
}
}
void init_op(zend_op *op TSRMLS_DC)
{
- memset(op, 0, sizeof(zend_op));
+ memset(op, 0, ZEND_MM_ALIGNED_SIZE(sizeof(zend_op)));
op->lineno = CG(zend_lineno);
SET_UNUSED(op->result);
}
zend_op *get_next_op(zend_op_array *op_array TSRMLS_DC)
@@ -323,11 +323,11 @@
}
zend_brk_cont_element *get_next_brk_cont_element(zend_op_array
*op_array)
{
op_array->last_brk_cont++;
- op_array->brk_cont_array = erealloc(op_array->brk_cont_array,
sizeof(zend_brk_cont_element)*op_array->last_brk_cont);
+ op_array->brk_cont_array = erealloc(op_array->brk_cont_array,
ZEND_MM_ALIGNED_SIZE(sizeof(zend_brk_cont_element))*op_array->last_brk_cont);
return &op_array->brk_cont_array[op_array->last_brk_cont-1];
}
static void zend_update_extended_info(zend_op_array *op_array
TSRMLS_DC)
{
@@ -372,11 +372,11 @@
if (CG(compiler_options) & ZEND_COMPILE_HANDLE_OP_ARRAY) {
zend_llist_apply_with_argument(&zend_extensions,
(llist_apply_with_arg_func_t) zend_extension_op_array_handler, op_array
TSRMLS_CC);
}
if (!(op_array->fn_flags & ZEND_ACC_INTERACTIVE) && op_array->size !=
op_array->last) {
- op_array->opcodes = (zend_op *) erealloc(op_array->opcodes,
sizeof(zend_op)*op_array->last);
+ op_array->opcodes = (zend_op *) erealloc(op_array->opcodes,
ZEND_MM_ALIGNED_SIZE(sizeof(zend_op))*op_array->last);
op_array->size = op_array->last;
}
opline = op_array->opcodes;
end = opline + op_array->last;
diff -U5 ./php-5.3.0/Zend/zend_vm_def.h
./php-5.3.0D/Zend/zend_vm_def.h
--- ./php-5.3.0/Zend/zend_vm_def.h 2009-06-07 11:46:51.000000000 -0400
+++ ./php-5.3.0D/Zend/zend_vm_def.h 2009-08-18 12:24:29.649999000
-0400
@@ -4253,11 +4253,11 @@
zend_uint catch_op_num;
int catched = 0;
zval restored_error_reporting;
void **stack_frame = (void**)EX(Ts) +
- (sizeof(temp_variable) * EX(op_array)->T) / sizeof(void*);
+ (ZEND_MM_ALIGNED_SIZE(sizeof(temp_variable)) * EX(op_array)->T)
/
ZEND_MM_ALIGNED_SIZE(sizeof(void*));
while (zend_vm_stack_top(TSRMLS_C) != stack_frame) {
zval *stack_zval_p = zend_vm_stack_pop(TSRMLS_C);
zval_ptr_dtor(&stack_zval_p);
}
diff -U5 ./php-5.3.0/Zend/zend_vm_execute.h
./php-5.3.0D/Zend/zend_vm_execute.h
--- ./php-5.3.0/Zend/zend_vm_execute.h 2009-06-07 11:46:51.000000000
-0400
+++ ./php-5.3.0D/Zend/zend_vm_execute.h 2009-08-18 12:35:01.390003000
-0400
@@ -50,16 +50,16 @@
EG(in_execution) = 1;
zend_vm_enter:
/* Initialize execute_data */
execute_data = (zend_execute_data *)zend_vm_stack_alloc(
- sizeof(zend_execute_data) +
- sizeof(zval**) * op_array->last_var * (EG(active_symbol_table)
? 1 :
2) +
- sizeof(temp_variable) * op_array->T TSRMLS_CC);
+ ZEND_MM_ALIGNED_SIZE(sizeof(zend_execute_data)) +
+ ZEND_MM_ALIGNED_SIZE(sizeof(zval**)) * op_array->last_var *
(EG(active_symbol_table) ? 1 : 2) +
+ ZEND_MM_ALIGNED_SIZE(sizeof(temp_variable)) * op_array->T
TSRMLS_CC);
- EX(CVs) = (zval***)((char*)execute_data +
sizeof(zend_execute_data));
- memset(EX(CVs), 0, sizeof(zval**) * op_array->last_var);
+ EX(CVs) = (zval***)((char*)execute_data +
ZEND_MM_ALIGNED_SIZE(sizeof(zend_execute_data)));
+ memset(EX(CVs), 0, ZEND_MM_ALIGNED_SIZE(sizeof(zval**)) *
op_array->last_var);
EX(Ts) = (temp_variable *)(EX(CVs) + op_array->last_var *
(EG(active_symbol_table) ? 1 : 2));
EX(fbc) = NULL;
EX(called_scope) = NULL;
EX(object) = NULL;
EX(old_error_reporting) = NULL;
@@ -601,11 +601,11 @@
zend_uint catch_op_num;
int catched = 0;
zval restored_error_reporting;
void **stack_frame = (void**)EX(Ts) +
- (sizeof(temp_variable) * EX(op_array)->T) / sizeof(void*);
+ (ZEND_MM_ALIGNED_SIZE(sizeof(temp_variable)) * EX(op_array)->T)
/
ZEND_MM_ALIGNED_SIZE(sizeof(void*));
while (zend_vm_stack_top(TSRMLS_C) != stack_frame) {
zval *stack_zval_p = zend_vm_stack_pop(TSRMLS_C);
zval_ptr_dtor(&stack_zval_p);
}
diff -U5 ./php-5.3.0/Zend/zend_vm_execute.skl
./php-5.3.0D/Zend/zend_vm_execute.skl
--- ./php-5.3.0/Zend/zend_vm_execute.skl 2008-06-11 09:18:41.000000000
-0400
+++ ./php-5.3.0D/Zend/zend_vm_execute.skl 2009-08-18 12:33:57.330001000
-0400
@@ -16,16 +16,16 @@
EG(in_execution) = 1;
zend_vm_enter:
/* Initialize execute_data */
execute_data = (zend_execute_data *)zend_vm_stack_alloc(
- sizeof(zend_execute_data) +
- sizeof(zval**) * op_array->last_var * (EG(active_symbol_table)
? 1 :
2) +
- sizeof(temp_variable) * op_array->T TSRMLS_CC);
+ ZEND_MM_ALIGNED_SIZE(sizeof(zend_execute_data)) +
+ ZEND_MM_ALIGNED_SIZE(sizeof(zval**)) * op_array->last_var *
(EG(active_symbol_table) ? 1 : 2) +
+ ZEND_MM_ALIGNED_SIZE(sizeof(temp_variable)) * op_array->T
TSRMLS_CC);
- EX(CVs) = (zval***)((char*)execute_data +
sizeof(zend_execute_data));
- memset(EX(CVs), 0, sizeof(zval**) * op_array->last_var);
+ EX(CVs) = (zval***)((char*)execute_data +
ZEND_MM_ALIGNED_SIZE(sizeof(zend_execute_data)));
+ memset(EX(CVs), 0, ZEND_MM_ALIGNED_SIZE(sizeof(zval**)) *
op_array->last_var);
EX(Ts) = (temp_variable *)(EX(CVs) + op_array->last_var *
(EG(active_symbol_table) ? 1 : 2));
EX(fbc) = NULL;
EX(called_scope) = NULL;
EX(object) = NULL;
EX(old_error_reporting) = NULL;
------------------------------------------------------------------------
[2009-08-18 17:17:46] sriram dot natarajan at gmail dot com
no, I don't think it is just an issue with the allocation size.
during php script execution, execute API within Zend/zend_vm_execute.h
will allocate space from
zend_vm_stack (aka EG(argument_stack) for execution of a class or
function. however, the EG(argument_stack)->top is not always 8 byte
aligned because of the different number of arguments pushed etc.
the reason being that the offset (EG(argument_stack)->top) constantly
refers to the top of the stack and this is not always 8 byte aligned.
for example, I was able to move much further by aligning the address
returned from EX(CVs) and EX(Ts).
unfortunately,my day job is going to keep me away from this interesting
bug until next week (till thursday). hopefully, i will be able to come
back to this before 5.3.1
------------------------------------------------------------------------
[2009-08-18 12:12:44] dmda at yandex dot ru
another example is code below
execute_data = (zend_execute_data *)zend_vm_stack_alloc(
sizeof(zend_execute_data) +
sizeof(zval**) * op_array->last_var * (EG(active_symbol_table)
? 1 :
2) +
sizeof(temp_variable) * op_array->T TSRMLS_CC);
If I got it right, it tries to allocate multiple entities in the same
chunk of memory under zend_vm_stack's control.
If that's the case, all the entities should be aligned separately, like
below
execute_data = (zend_execute_data *)zend_vm_stack_alloc(
ZEND_MM_ALIGNED_SIZE(sizeof(zend_execute_data)) +
ZEND_MM_ALIGNED_SIZE(sizeof(zval**)) * op_array->last_var *
(EG(active_symbol_table) ? 1 : 2) +
ZEND_MM_ALIGNED_SIZE(sizeof(temp_variable)) * op_array->T
TSRMLS_CC);
------------------------------------------------------------------------
[2009-08-18 10:42:27] dmda at yandex dot ru
everything is correct, except 8byte requirement.
The alignment requirement for all CPUs (including x86, of course) is
per following:
to access data of n bytes, its address should start with n-byte
boundary, in particular
-to access 8byte data (such as 64bit long long) the address should be
aligned to 8byte boundary (for example 0x12340 and 0x12348 are correct
addresses, while 0x12344 and 0x12342 are not)
-to access 4byte data (such as 32bit integer) the address must be
aligned to 4byte (0x12340, 0x12344, and 0x12348 are correct addresses,
while 0x12342 is not)
-to access 2byte data (such as 16bit short) the address must be aligned
to 2byte (0x12340, 0x12342, 0x12344, and 0x12348 are correct)
in case of x86 CPU, it has 2 modes: it can raise interrupt or issue an
extra bus cycle to fetch the remaining data. Under most OSes x86 is
configured to issue bus cycle. As of the other CPUs they raise Bus
error.
How to fix:
the code below taken from zend_execute.h is just an example of bad
practice in address calculation:
size = (size + (sizeof(void*) - 1)) / sizeof(void*);
ZEND_VM_STACK_GROW_IF_NEEDED((int)size);
ret = (void*)EG(argument_stack)->top;
EG(argument_stack)->top += size;
it takes into account size of the addres (which is 32bit in my case),
but target data size may be 64bit.
Interestingly enough that an APPROPRIATE code can also be found there,
see Zend.m4 with ZEND_MM_ALIGNMENT, so it would be enough to replace
sizeof(void*) with ZEND_MM_ALIGNMENT :)
------------------------------------------------------------------------
The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at
http://bugs.php.net/48668
--
Edit this bug report at http://bugs.php.net/?id=48668&edit=1
