Hi, Lubo

The VLAN 4095 is a "reserved" tag number, not invalid. I think we can directly 
delete the duplicate items in the variable data.

Siyuan

> -----Original Message-----
> From: Zhang, Lubo
> Sent: Tuesday, February 23, 2016 10:48 AM
> To: edk2-devel@lists.01.org
> Cc: Fu, Siyuan <siyuan...@intel.com>; Ye, Ting <ting...@intel.com>; Wu,
> Jiaxin <jiaxin...@intel.com>
> Subject: [patch] MdeModulePkg:Fix a robustness issue of Mnp Driver
> 
> Duplicate items in VLAN variable will cause MNP driver binding start function
> fall into infinite loop,so we should check it's content before using it.
> 
> Cc: Fu Siyuan <siyuan...@intel.com>
> Cc: Ye Ting <ting...@intel.com>
> Cc: Wu Jiaxin <jiaxin...@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Zhang Lubo <lubo.zh...@intel.com>
> ---
>  MdeModulePkg/Universal/Network/MnpDxe/MnpDriver.c | 23
> +++++++++++---------
>  MdeModulePkg/Universal/Network/MnpDxe/MnpImpl.h   |  1 +
>  MdeModulePkg/Universal/Network/MnpDxe/MnpVlan.c   | 26
> +++++++++++++++++++++++
>  3 files changed, 40 insertions(+), 10 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Network/MnpDxe/MnpDriver.c
> b/MdeModulePkg/Universal/Network/MnpDxe/MnpDriver.c
> index 2ddcec8..cc742ea 100644
> --- a/MdeModulePkg/Universal/Network/MnpDxe/MnpDriver.c
> +++ b/MdeModulePkg/Universal/Network/MnpDxe/MnpDriver.c
> @@ -238,21 +238,24 @@ MnpDriverBindingStart (
>    //
>    // Create MNP service data for each VLAN
>    //
>    MnpDeviceData->NumberOfVlan = NumberOfVlan;
>    for (Index = 0; Index < NumberOfVlan; Index++) {
> -    MnpServiceData = MnpCreateServiceData (
> -                       MnpDeviceData,
> -                       VlanVariable[Index].Bits.Vid,
> -                       (UINT8) VlanVariable[Index].Bits.Priority
> -                       );
> -
> -    if (MnpServiceData == NULL) {
> -      Status = EFI_OUT_OF_RESOURCES;
> -
> -      goto Exit;
> +    if (VlanVariable[Index].Bits.Vid != MNP_INVALID_VLAN_IDENTIFIER) {
> +      MnpServiceData = MnpCreateServiceData (
> +                         MnpDeviceData,
> +                         VlanVariable[Index].Bits.Vid,
> +                         (UINT8) VlanVariable[Index].Bits.Priority
> +                         );
> +
> +      if (MnpServiceData == NULL) {
> +        Status = EFI_OUT_OF_RESOURCES;
> +
> +        goto Exit;
> +      }
>      }
> +
>    }
> 
>  Exit:
>    if (VlanVariable != NULL) {
>      FreePool (VlanVariable);
> diff --git a/MdeModulePkg/Universal/Network/MnpDxe/MnpImpl.h
> b/MdeModulePkg/Universal/Network/MnpDxe/MnpImpl.h
> index 51391af..7ccc911 100644
> --- a/MdeModulePkg/Universal/Network/MnpDxe/MnpImpl.h
> +++ b/MdeModulePkg/Universal/Network/MnpDxe/MnpImpl.h
> @@ -17,10 +17,11 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF
> ANY KIND, EITHER EXPRESS OR IMPLIED.
>  #define _MNP_IMPL_H_
> 
>  #include "MnpDriver.h"
> 
>  #define NET_ETHER_FCS_SIZE            4
> +#define MNP_INVALID_VLAN_IDENTIFIER  4095
> 
>  #define MNP_SYS_POLL_INTERVAL         (10 * TICKS_PER_MS)   // 10
> milliseconds
>  #define MNP_TIMEOUT_CHECK_INTERVAL    (50 * TICKS_PER_MS)   // 50
> milliseconds
>  #define MNP_MEDIA_DETECT_INTERVAL     (500 * TICKS_PER_MS)  // 500
> milliseconds
>  #define MNP_TX_TIMEOUT_TIME           (500 * TICKS_PER_MS)  // 500
> milliseconds
> diff --git a/MdeModulePkg/Universal/Network/MnpDxe/MnpVlan.c
> b/MdeModulePkg/Universal/Network/MnpDxe/MnpVlan.c
> index 6e14c1f..b24a50c 100644
> --- a/MdeModulePkg/Universal/Network/MnpDxe/MnpVlan.c
> +++ b/MdeModulePkg/Universal/Network/MnpDxe/MnpVlan.c
> @@ -228,10 +228,35 @@ MnpInsertVlanTag (
>    VlanTci->Bits.Cfi      = VLAN_TCI_CFI_CANONICAL_MAC;
>    VlanTci->Bits.Priority = MnpServiceData->Priority;
>    VlanTci->Uint16        = HTONS (VlanTci->Uint16);
>  }
> 
> +/**
> +  Check VLAN configuration variable and mark it if there has identical Vlan 
> ID.
> +
> +  @param[in]      Buffer             Pointer to the buffer contains the 
> requested
> +                                     array of VLAN_TCI.
> +  @param[in]      NumberOfVlan       Pointer to number of VLAN to be
> returned.
> +
> +**/
> +VOID
> +MnpCheckVlanVariable (
> +  IN     VLAN_TCI          *Buffer,
> +  IN     UINTN             NumberOfVlan
> +  )
> +{
> +  UINTN             Index;
> +  UINTN             Injex;
> +
> +  for (Index = 0; Index < NumberOfVlan; Index++) {
> +   for (Injex = Index + 1; Injex < NumberOfVlan; Injex++) {
> +     if (Buffer[Index].Bits.Vid == Buffer[Injex].Bits.Vid) {
> +       Buffer[Injex].Bits.Vid = MNP_INVALID_VLAN_IDENTIFIER;
> +     }
> +   }
> +  }
> +}
> 
>  /**
>    Get VLAN configuration variable.
> 
>    @param[in]       MnpDeviceData      Pointer to the MNP device context data.
> @@ -292,10 +317,11 @@ MnpGetVlanVariable (
>      return Status;
>    }
> 
>    *NumberOfVlan = BufferSize / sizeof (VLAN_TCI);
>    *VlanVariable = Buffer;
> +  MnpCheckVlanVariable (Buffer,*NumberOfVlan);
> 
>    return Status;
>  }
> 
> 
> --
> 1.9.5.msysgit.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to