Bug 1167069 - Segmentation fault when calling libvirt_network_set_active
Summary: Segmentation fault when calling libvirt_network_set_active
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Virtualization Tools
Classification: Community
Component: libvirt-php
Version: unspecified
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Michal Privoznik
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-11-23 07:14 UTC by James
Modified: 2016-09-21 12:22 UTC (History)
2 users (show)

Fixed In Version: libvirt-php-0.5.2-39-gc725824
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-09-21 12:22:40 UTC


Attachments (Terms of Use)

Description James 2014-11-23 07:14:07 UTC
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:

Comment 2 James 2014-11-23 08:19:33 UTC
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).

Comment 3 James 2014-11-23 08:46:15 UTC
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

Comment 4 James 2014-11-23 08:49:07 UTC
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);

Comment 5 James 2014-11-23 11:19:58 UTC
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)

Comment 6 Cole Robinson 2016-03-23 20:13:28 UTC
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@redhat.com , but I'm CCing him here, maybe he'll get to it first :)

Comment 7 Michal Privoznik 2016-09-21 12:22:40 UTC
I believe this is fixed by these two commits (and possibly others too)

commit cc8190ba60f911e04eb10a0e28dc3c9123e15bae
Author:     Remi Collet <fedora@famillecollet.com>
AuthorDate: Wed Apr 13 12:13:08 2016 -0400
Commit:     Michal Privoznik <mprivozn@redhat.com>
CommitDate: Thu Apr 14 18:24:36 2016 +0200

    add missing arginfo
    
    Signed-off-by: Michal Privoznik <mprivozn@redhat.com>


commit 9f355ebed7a217669269c1a4016329f7411e356b
Author:     Michal Novotny <minovotn@redhat.com>
AuthorDate: Fri Jul 19 13:20:46 2013 +0200
Commit:     Michal Novotny <minovotn@redhat.com>
CommitDate: Fri Jul 19 13:20:46 2013 +0200

    Fix libvirt_network_set_active


Note You need to log in before you can comment on or make changes to this bug.