Description of problem: Calling libvirt_network_set_active causes a segmentation fault when the second parameter is passed. Version-Release number of selected component (if applicable): 0.4.8 How reproducible: Every time. Steps to Reproduce: <?php $conn = libvirt_connect('connection_uri', false); $network = libvirt_network_get($conn, 'network_name_here'); // This will return an error stating that it expected 2 parameters (the // documentation states it only accepts 1 parameter, but that doesn't // make much sense for this function call). libvirt_network_set_active($network); // Both of these calls will cause a segmentation fault. libvirt_network_set_active($network, true); libvirt_network_set_active($network, 1); ?> Actual results: Segmentation fault when calling libvirt_network_set_active. Expected results: Network active status should be set as intended. Additional info:
According to GDB, the segmentation fault occurs in this line: http://libvirt.org/git/?p=libvirt-php.git;a=blob;f=src/libvirt-php.c;h=f614002533993d706cf716459a0fe7bc988e3b88;hb=f614002533993d706cf716459a0fe7bc988e3b88#l7571
I'm pretty sure this is a bug in PHP. After modifying the libvirt to do some debugging, expanding constants and adding some debugging printf's, I ended up with this: -------------------------------------------- PHP_FUNCTION(libvirt_network_set_active) { php_libvirt_network *network = NULL; zval *znetwork; unsigned int act = 0; reset_error(TSRMLS_C); if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "rl", &znetwork, &act) == FAILURE) { set_error("Invalid arguments" TSRMLS_CC); RETURN_FALSE; } if (znetwork == NULL) { printf("znetwork is NULL.\n"); } else { printf("znetwork is not NULL.\n"); } zend_uchar tt; tt = znetwork->type; printf("Type of resource is %d\n", tt); printf("About to call ZEND_FETCH_RESOURCE...\n"); network = (php_libvirt_network*)zend_fetch_resource(&znetwork TSRMLS_CC, -1, PHP_LIBVIRT_NETWORK_RES_NAME, NULL, 1, le_libvirt_network); if (!network) { RETURN_FALSE; } if (network == NULL || network->network == NULL) { RETURN_FALSE; } if ((act != 0) && (act != 1)) { set_error("Invalid network activity state" TSRMLS_CC); RETURN_FALSE; } -------------------------------------------- When running under gdb, I get a segmentation fault on the "tt = znetwork->type;" line, indicating that the value of znetwork isn't a valid pointer. It writes out "znetwork is not NULL." before this, indicating that PHP is giving back an invalid pointer. Interestingly enough, I can fix the issue by changing the parameter declaration to: -------------------------------------------- PHP_FUNCTION(libvirt_network_set_active) { php_libvirt_network *network = NULL; zval *znetwork; zend_bool act = 0; reset_error(TSRMLS_C); if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "rb", &znetwork, &act) == FAILURE) { set_error("Invalid arguments" TSRMLS_CC); RETURN_FALSE; } -------------------------------------------- This doesn't crash, and znetwork has a valid value, all because I changed the second parameter from long to boolean ("rl" to "rb"). I verified with other functions that "rl" looks like it's being used correctly (storing in the correct type), so I'm not sure why PHP is returning an invalid pointer here, and why it only seems to apply to this function (no other networking functions are affected).
Filed a bug in PHP upstream in the event that this is a bug in the PHP parameter parsing: https://bugs.php.net/bug.php?id=68484
Here's a patch for the "change parameter type to bool" fix, although it'd be much nicer to discover why "rl" causes this issue in the first place: --- libvirt.c.old 2014-11-23 19:48:03.350647661 +1100 +++ libvirt.c 2014-11-23 19:48:10.934806105 +1100 @@ -7564,9 +7564,9 @@ { php_libvirt_network *network; zval *znetwork; - int act = 0; + zend_bool act = 0; - GET_NETWORK_FROM_ARGS("rl",&znetwork,&act); + GET_NETWORK_FROM_ARGS("rb",&znetwork,&act); if ((act != 0) && (act != 1)) { set_error("Invalid network activity state" TSRMLS_CC);
According to https://bugs.php.net/bug.php?id=68484 it's because the size of int and long can differ. Because the arguments are pushed in reverse order, the size difference causes corruption of the network parameter. The following functions are presumably susceptible to the same issue, because they use an int or unsigned int variable for accepting one or more long parameters: libvirt_domain_get_screenshot_api (screen) libvirt_domain_get_screenshot (scancode) libvirt_domain_send_pointer_event (pos_x, pos_y, clicked) libvirt_domain_change_vcpus (numCpus only) libvirt_domain_new (memMB,maxmemMB,vcpus) libvirt_storagevolume_delete (flags) libvirt_domain_snapshot_delete (flags)
Thanks for the detailed info, sorry this never received a response. If you're still interested, the best way to get this fixed is to send a patch to libvir-list and CC mprivozn , but I'm CCing him here, maybe he'll get to it first :)
I believe this is fixed by these two commits (and possibly others too) commit cc8190ba60f911e04eb10a0e28dc3c9123e15bae Author: Remi Collet <fedora> AuthorDate: Wed Apr 13 12:13:08 2016 -0400 Commit: Michal Privoznik <mprivozn> CommitDate: Thu Apr 14 18:24:36 2016 +0200 add missing arginfo Signed-off-by: Michal Privoznik <mprivozn> commit 9f355ebed7a217669269c1a4016329f7411e356b Author: Michal Novotny <minovotn> AuthorDate: Fri Jul 19 13:20:46 2013 +0200 Commit: Michal Novotny <minovotn> CommitDate: Fri Jul 19 13:20:46 2013 +0200 Fix libvirt_network_set_active