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
