[ovs-dev] [PATCH 12/14] datapath-windows: Add netlink command vport delete

[Alin Serdean]

Good catch Nithin thank you for folding it up!

Thanks,
Alin.

-----Mesaj original-----
De la: Nithin Raju [mailto:nithin at vmware.com] 
Trimis: Wednesday, October 8, 2014 12:29 AM
Către: Samuel Ghinet; dev at openvswitch.org
Cc: Alin Serdean; Eitan Eliahu; Ankur Sharma
Subiect: RE: [PATCH 12/14] datapath-windows: Add netlink command vport delete

> +    if (vport->hvDeleted || OvsIsTunnelVportType(vport->ovsType)) {
> +        /*
> +         * The associated hyper-v switch port is not in created state, or,
> +         * there is no hyper-v switch port counterpart (for logical ports).
> +         * This means that this datapath port is not mapped to a living
> +         * hyper-v switc hport. We can destroy and remove the vport from the
> +         * list.
> +        */
> +        OvsRemoveAndDeleteVport(gOvsSwitchContext, vport);
> +    } else {
> +        /* The associated hyper-v switch port is in the created state, and
> the
> +         * datapath port is mapped to a living hyper-v switch port. We
> cannot
> +         * destroy the vport and cannot remove it from the list of vports.
> +         * Instead, we mark the datapath (ovs) part of the vport as
> +         * "not created", i.e. we set vport->portNo =
> OVS_PORT_NUMBER_INVALID.
> +        */
> +        vport->portNo = OVS_DPPORT_NUMBER_INVALID;
> +        vport->ovsName[0] = '\0';
> +    }

Alin,
There's a bug here that leads to memory corruption. When a vport is deleted, we are invalidating 'vport->portNo' and 'vport->ovsName'. But, we are not removing it from the 'ovsNameHashArray', and 'portNoHashArray'. If the same Vport is added back again, it gets re-inserted into the same list leading to pointer corruption.

Pls. fold in the following fix:
diff --git a/datapath-windows/ovsext/Datapath.c b/datapath-windows/ovsext/Datapath.c
index 1af207a..acd9dbb 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -2055,6 +2055,8 @@ OvsDeleteVportCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
          * Instead, we mark the datapath (ovs) part of the vport as
          * "not created", i.e. we set vport->portNo = OVS_PORT_NUMBER_INVALID.
         */
+        RemoveEntryList(&vport->ovsNameLink);
+        RemoveEntryList(&vport->portNoLink);
         vport->portNo = OVS_DPPORT_NUMBER_INVALID;
         vport->ovsName[0] = '\0';

thanks,
-- Nithin