On Sep 30, 2014, at 8:03 AM, Samuel Ghinet <sghi...@cloudbasesolutions.com>
wrote:
> If the hyper-v switch port type is external, then the function
> OvsInitConfiguredSwitchNics allocates a vport, and if allocation
> succeeds, it procedes with the initialization. However, at this
> point, virtPort may happen to be null, but check against virtPort
> was not made - this means that if virtPort was null, later on its
> fields would try to be accessed.
>
> This patch adds a check for virtPort as well, so that the fields of
> virtPort will not be accessed if virtPort is NULL.
>
> Signed-off-by: Samuel Ghinet <sghi...@cloudbasesolutions.com>
> ---
> datapath-windows/ovsext/Vport.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c
> index d90fd2b..1e05d60 100644
> --- a/datapath-windows/ovsext/Vport.c
> +++ b/datapath-windows/ovsext/Vport.c
> @@ -857,10 +857,9 @@ OvsInitConfiguredSwitchNics(POVS_SWITCH_CONTEXT
> switchContext)
>
> if (nicParam->NicType == NdisSwitchNicTypeExternal &&
> nicParam->NicIndex != 0) {
> - POVS_VPORT_ENTRY virtVport =
> - (POVS_VPORT_ENTRY)switchContext->externalVport;
> + POVS_VPORT_ENTRY virtVport = switchContext->externalVport;
> vport = OvsAllocateVport();
> - if (vport) {
> + if (vport && virtVport) {
> OvsInitPhysNicVport(vport, virtVport, nicParam->NicIndex);
> status = OvsInitVportCommon(switchContext, vport);
> if (status != NDIS_STATUS_SUCCESS) {
Well, this logic is wrong for the following reasons:
1. You are leaking memory. The memory allocated for vport is not released.
2. You are not updating switchContext->externalVport to be vport, but instead
copying out values from vport to switchContext->externalVport. This is not
wrong as such, but ideally, we just want switchContext->externalVport to just
point to the correct vport.
I don't think we need this patch, since there's a slightly intricate port/nic
create sequence with external ports. We can consider patch after the first 13
are checked in.
To make sure we are not accessing a NULL pointer, we can add an
ASSERT(virtVport); and an XXX comment and deal with it as the code stabilizes.
thanks,
Nithin
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
