On 02/19/2014 09:54 AM, Michal Privoznik wrote:

Currently, networkRunHook() is called in networkAllocateActualDevice and friends. These functions, however, doesn't necessarily work on networks,

s/doesn't/don't/

For example, if domain's interface is defined in this fashion:

<interface type='bridge'> <mac address='52:54:00:0b:3b:16'/> <source bridge='virbr1'/> <model type='rtl8139'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/> </interface>

The networkAllocateActualDevice jumps directly onto 'validate' label as the interface is not type of 'network'. Hence, @network is left initialized to NULL and networkRunHook(network, ...) is called. One of the things that the hook function does is dereference @network. Soupir.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/network/bridge_driver.c | 6 ++++++ 1 file changed, 6 insertions(+)

I'll give an ACK to the change... Although I will also note that while the patch does resolve the current issue and perhaps protect against future errant callers... Currently the only caller that could get into the hook function with network == NULL is networkAllocateActualDevice() Other places that would end up using the hook will bail when iface->type != VIR_DOMAIN_NET_TYPE_NETWORK or when/by checking virNetworkObjIsActive(network). Your other option would seem to be checking "(iface->type == VIR_DOMAIN_NET_TYPE_NETWORK)" before calling the hook in the validate: section. John

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index ee264b9..a6c719d 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -150,6 +150,12 @@ networkRunHook(virNetworkObjPtr network, int ret = -1;

if (virHookPresent(VIR_HOOK_DRIVER_NETWORK)) { + if (!network) { + VIR_DEBUG("Not running hook as @network is NULL"); + ret = 0; + goto cleanup; + } + virBufferAddLit(&buf, "<hookData>\n"); virBufferAdjustIndent(&buf, 2); if (virNetworkDefFormatBuf(&buf, network->def, 0) < 0)