Segfault 5.5.6

[joeyhub]

Program received signal SIGSEGV, Segmentation fault.
0x00000000006e51d9 in gc_zval_possible_root ()
(gdb) bt
#0 0x00000000006e51d9 in gc_zval_possible_root ()
#1 0x00000000006d3628 in zend_hash_destroy ()
#2 0x00000000006c482b in _zval_dtor_func ()
#3 0x00000000006b694a in _zval_ptr_dtor ()
#4 0x00007fffee000d8a in msgpack_unserialize_var_destroy (var_hashx=var_hashx@entry=0x7fffffff1170, err=err@entry=1 '\001')

at /tmp/pear/temp/msgpack/msgpack_unpack.c:331

#5 0x00007fffedffb54a in php_msgpack_unserialize (return_value=return_value@entry=0x7fffedb82178, str=, str_len=2363631)

at /tmp/pear/temp/msgpack/msgpack.c:268

#6 0x00007fffedffb66d in zif_msgpack_unserialize (ht=, return_value=0x7fffedb82178, return_value_ptr=,

this_ptr=<optimized out>, return_value_used=<optimized out>) at /tmp/pear/temp/msgpack/msgpack.c:332

#7 0x000000000076d471 in ?? ()
#8 0x00000000007272e7 in execute ()
#9 0x00000000006c714c in zend_execute_scripts ()
#10 0x0000000000666fd3 in php_execute_script ()
#11 0x000000000076fe03 in ?? ()
#12 0x0000000000431c8f in ?? ()
#13 0x00007ffff503eead in __libc_start_main () from /lib/x86_64-linux-gnu/libc.so.6
#14 0x0000000000431d25 in _start ()

[laruence]

are you able to provide a small reproduce script?

[joeyhub]

I am trying to ;). I have 5.5.5 and 5.5.6 compiled. Will update when I find a way to trigger it again.

[joeyhub]

mkdir issue_49
cd issue_49

wget https://pecl.php.net/get/msgpack-0.5.{5,6}.tgz
for v in 5 6
do
 tar -xvzf msgpack-0.5.$v.tgz
 cd msgpack-0.5.$v
 phpize
 ./configure
 make
 cd ..
 php -n -dextension=msgpack-0.5.$v/modules/msgpack.so\
  -r 'echo msgpack_pack([["item"=>["obj"=>(object)["y" => "YYYYYYY"],"str" =>["x" => "XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX"]]]]);'\
  > msgpack$v
done

php -n -dextension=msgpack-0.5.5/modules/msgpack.so\
 -r "msgpack_unpack(file_get_contents("msgpack6"));"

Enjoy :).

[joeyhub]

In some cases I also get a message about bad msgpack structure.

For example... ["item"=>["str" =>["x" => "XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX"]]] gives:

Warning: msgpack Parse error in Command line code on line 1

[joeyhub]

It's the str8, you added encode support in the last revision. This shaves a byte for strings < 256.

# cat msgpack5 | xxd
0000000: 9181 a469 7465 6d82 a36f 626a 82c0 a873  ...item..obj...s
0000010: 7464 436c 6173 73a1 79a7 5959 5959 5959  tdClass.y.YYYYYY
0000020: 59a3 7374 7281 a178 da00 3658 5858 5858  Y.str..x..6XXXXX
0000030: 5858 5858 5858 5858 5858 5858 5858 5858  XXXXXXXXXXXXXXXX
0000040: 5858 5858 5858 5858 5858 5858 5858 5858  XXXXXXXXXXXXXXXX
0000050: 5858 5858 5858 5858 5858 5858 5858 5858  XXXXXXXXXXXXXXXX
0000060: 58                                       X
# cat msgpack6 | xxd
0000000: 9181 a469 7465 6d82 a36f 626a 82c0 a873  ...item..obj...s
0000010: 7464 436c 6173 73a1 79a7 5959 5959 5959  tdClass.y.YYYYYY
0000020: 59a3 7374 7281 a178 d936 5858 5858 5858  Y.str..x.6XXXXXX
0000030: 5858 5858 5858 5858 5858 5858 5858 5858  XXXXXXXXXXXXXXXX
0000040: 5858 5858 5858 5858 5858 5858 5858 5858  XXXXXXXXXXXXXXXX
0000050: 5858 5858 5858 5858 5858 5858 5858 5858  XXXXXXXXXXXXXXXX

da (str16) to d9 (str8)

So 0.5.5 compatibility is broken but also it should not really segfault either :(.

When making such a change I really think it would be a good idea to attribute it to a less minor version component.

That is, 5.5.6 should be 5.6.0.

[Sean-Der]

Hey @laruence when looking at implementing this I think I have two options. I need to add a conditional on https://github.com/msgpack/msgpack-php/blob/master/msgpack/pack_template.h#L733 I have two ideas on how I could accomplish this. Either way I have to edit this 'upstream' header file. (I would like to avoid this, but it seems like other implementations are doing the same thing?)

I could either make msgpack_pack_user a struct that contains the zend_string and a boolean use_str8_serialization
OR
just put a call to MSGPACK_G(use_str8_serialization)

Also what do you think of the STD_PHP_INI_BOOLEAN name use_str8_serialization

[Sean-Der]

Hey!

The msgpack spec calls for a compatibility mode so serialization doesn't create data that older deserializers can't handle.
You can see that in action here

@joeyhub
So the segfault is no good, but there isn't anything we can do in the old versions. I will however check and make sure that this segfault doesn't occur for any other 'first bytes'!

@mattheworiordan
Just a heads up this is done! Looking at the issue you posted you hopefully should be unblocked, but also super happy to tackle anything you throw my way :)

thanks.