Re: libzip-1.3.1 released -segfault

[Remi Collet <remi%fedoraproject.org@localhost>]

Le 19/11/2017 à 16:41, Thomas Klausner a écrit :
> 
> Hi!
> 
> We've just released libzip-1.3.1. What's new?

Building PHP against this new version raise a segfault during the test
suite.


(gdb) bt
#0  zip_source_free (src=0x7478742e) at zip_source_free.c:46
#1  0x00007fffec9212fe in zip_source_free (src=0x555555c9d530) at
zip_source_free.c:68
#2  0x00007fffec919b97 in zip_discard (za=0x555555c9bef0) at
zip_discard.c:54
#3  0x00007fffecb34271 in c_ziparchive_close (execute_data=<optimized
out>, return_value=0x7ffff3814150) at /work/GIT/php_zip/php7/php_zip.c:1540
#4  0x0000555555827bce in ZEND_DO_FCALL_SPEC_HANDLER ()
#5  0x00005555557e242b in execute_ex ()
#6  0x0000555555836c67 in zend_execute ()
#7  0x00005555557a1913 in zend_execute_scripts ()
#8  0x00005555557404d0 in php_execute_script ()
#9  0x000055555583891c in do_cli ()
#10 0x000055555561ff29 in main ()


Changes in libzip

diff -ru libzip-1.3.0/lib/zip_close.c libzip-1.3.1/lib/zip_close.c
--- libzip-1.3.0/lib/zip_close.c        2017-05-17 19:41:59.000000000 +0200
+++ libzip-1.3.1/lib/zip_close.c        2017-11-13 10:59:12.000000000 +0100
@@ -218,14 +218,13 @@

     if (error) {
        zip_source_rollback_write(za->src);
-       return -1;
     }

     _zip_progress_end(za->progress);

     zip_discard(za);

-    return 0;
+    return error ? -1 : 0;
 }


As I understand, the change in libzip was introduced to fix a memory
leak (za not discarded), but when this was properly managed in caller,
this now introduce a segfault (double call to zip_discard).


In PHP extension sources

        if ((err = zip_close(intern))) {
                php_error_docref(NULL, E_WARNING, "%s", zip_strerror(intern));
                zip_discard(intern);
        }


Of course, the fix in PHP can be trivial, but this affects all PHP
versions, released for years.


Notice: most people build PHP against bundled libzip, but this is now
discouraged, and only system library gives all features, and the plan is
to totally drop the bundled copy.

So for now, this mostly only affects some linux distributions


What do you think ?


Remi

Attachment: signature.asc
Description: OpenPGP digital signature