EmbeddedPkg/Lan9118Dxe: Add or fix input parameter checks

Add or fix checking of the input parameters of the functions that constitute
the EFI_SIMPLE_NETWORK_PROTOCOL interface provided by the LAN9118 driver.
In case of invalid calls, the returned error codes are now compliant with the
UEFI specificationi and the SCT tests checking for those error codes do not
fail anymore.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Reviewed-By: Olivier Martin <olivier.martin@arm.com>



git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@16239 6f19259b-4bc3-4df7-8a09-765794883524

1 files changed, 213 insertions, 103 deletions

diff --git a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c
index ec0175408a..625abb8e31 100644
--- a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c
+++ b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c
@@ -175,7 +175,7 @@ Lan9118DxeEntry (
 EFI_STATUS

 EFIAPI

 SnpStart (

-  IN        EFI_SIMPLE_NETWORK_PROTOCOL* Snp

+  IN  EFI_SIMPLE_NETWORK_PROTOCOL  *Snp

  )

 {

   // Check Snp instance

@@ -184,10 +184,9 @@ SnpStart (
   }

 

   // Check state

-  if ((Snp->Mode->State == EfiSimpleNetworkStarted) || (Snp->Mode->State == EfiSimpleNetworkInitialized)) {

+  if ((Snp->Mode->State == EfiSimpleNetworkStarted)    ||

+      (Snp->Mode->State == EfiSimpleNetworkInitialized)  ) {

     return EFI_ALREADY_STARTED;

-  } else if (Snp->Mode->State == EfiSimpleNetworkMaxState) {

-    return EFI_DEVICE_ERROR;

   }

 

   // Change state

@@ -211,7 +210,7 @@ SnpStop (
   }

 

   // Check state of the driver

-  if ((Snp->Mode->State == EfiSimpleNetworkStopped) || (Snp->Mode->State == EfiSimpleNetworkMaxState)) {

+  if (Snp->Mode->State == EfiSimpleNetworkStopped) {

     return EFI_NOT_STARTED;

   }

 

@@ -473,7 +472,7 @@ SnpShutdown (
     DEBUG ((EFI_D_WARN, "Warning: LAN9118 Driver not yet initialized\n"));

     return EFI_DEVICE_ERROR;

   } else if (Snp->Mode->State == EfiSimpleNetworkStopped) {

-    DEBUG ((EFI_D_WARN, "Warning: LAN9118 Driver in stopped state\n"));

+    DEBUG ((EFI_D_WARN, "Warning: LAN9118 Driver not started\n"));

     return EFI_NOT_STARTED;

   }

 

@@ -493,84 +492,149 @@ SnpShutdown (
   return EFI_SUCCESS;

 }

 

+/**

+  Enable and/or disable the receive filters of the LAN9118

+

+  Please refer to the UEFI specification for the precedence rules among the

+  Enable, Disable and ResetMCastFilter parameters.

+

+  @param[in]  Snp               A pointer to the EFI_SIMPLE_NETWORK_PROTOCOL

+                                instance.

+  @param[in]  Enable            A bit mask of receive filters to enable.

+  @param[in]  Disable           A bit mask of receive filters to disable.

+  @param[in]  ResetMCastFilter  Set to TRUE to reset the contents of the multicast

+                                receive filters on the network interface to

+                                their default values.

+  @param[in]  MCastFilterCnt    Number of multicast HW MAC addresses in the new

+                                MCastFilter list. This value must be less than or

+                                equal to the MCastFilterCnt field of

+                                EFI_SIMPLE_NETWORK_MODE. This field is optional if

+                                ResetMCastFilter is TRUE.

+  @param[in]  MCastFilter       A pointer to a list of new multicast receive

+                                filter HW MAC addresses. This list will replace

+                                any existing multicast HW MAC address list. This

+                                field is optional if ResetMCastFilter is TRUE.

+

+  @retval  EFI_SUCCESS            The receive filters of the LAN9118 were updated.

+  @retval  EFI_NOT_STARTED        The LAN9118 has not been started.

+  @retval  EFI_INVALID_PARAMETER  One or more of the following conditions is TRUE :

+                                  . This is NULL

+                                  . Multicast is being enabled (the

+                                    EFI_SIMPLE_NETWORK_RECEIVE_MULTICAST bit is set in

+                                    Enable, it is not set in Disable, and ResetMCastFilter

+                                    is FALSE) and MCastFilterCount is zero.

+                                  . Multicast is being enabled and MCastFilterCount is

+                                    greater than Snp->Mode->MaxMCastFilterCount.

+                                  . Multicast is being enabled and MCastFilter is NULL

+                                  . Multicast is being enabled and one or more of the

+                                    addresses in the MCastFilter list are not valid

+                                    multicast MAC addresses.

+  @retval  EFI_DEVICE_ERROR       The LAN9118 has been started but not initialized.

 

-/*

- *  UEFI ReceiveFilters() function

- *

- */

+**/

 EFI_STATUS

 EFIAPI

 SnpReceiveFilters (

-  IN        EFI_SIMPLE_NETWORK_PROTOCOL* Snp,

-  IN        UINT32 Enable,

-  IN        UINT32 Disable,

-  IN        BOOLEAN Reset,

-  IN        UINTN NumMfilter          OPTIONAL,

-  IN        EFI_MAC_ADDRESS *Mfilter  OPTIONAL

+  IN  EFI_SIMPLE_NETWORK_PROTOCOL  *Snp,

+  IN  UINT32                       Enable,

+  IN  UINT32                       Disable,

+  IN  BOOLEAN                      ResetMCastFilter,

+  IN  UINTN                        MCastFilterCnt  OPTIONAL,

+  IN  EFI_MAC_ADDRESS              *MCastFilter  OPTIONAL

   )

 {

-  UINT32 MacCSRValue;

-  UINT32 MultHashTableHigh;

-  UINT32 MultHashTableLow;

-  UINT32 Crc;

-  UINT8 BitToSelect;

-  UINT32 Count;

+  EFI_SIMPLE_NETWORK_MODE  *Mode;

+  UINT32                   MultHashTableHigh;

+  UINT32                   MultHashTableLow;

+  UINT32                   Count;

+  UINT32                   Crc;

+  UINT8                    HashValue;

+  UINT32                   MacCSRValue;

+  EFI_MAC_ADDRESS          *Mac;

 

-  MacCSRValue = 0;

-  MultHashTableHigh = 0;

-  MultHashTableLow = 0;

-  Crc = 0xFFFFFFFF;

-  BitToSelect = 0;

-  Count = 0;

+  // Check Snp Instance

+  if (Snp == NULL) {

+    return EFI_INVALID_PARAMETER;

+  }

+  Mode = Snp->Mode;

 

   // Check that driver was started and initialised

-  if (Snp->Mode->State == EfiSimpleNetworkStarted) {

+  if (Mode->State == EfiSimpleNetworkStarted) {

     DEBUG ((EFI_D_WARN, "Warning: LAN9118 Driver not initialized\n"));

     return EFI_DEVICE_ERROR;

-  } else if (Snp->Mode->State == EfiSimpleNetworkStopped) {

+  } else if (Mode->State == EfiSimpleNetworkStopped) {

     DEBUG ((EFI_D_WARN, "Warning: LAN9118 Driver in stopped state\n"));

     return EFI_NOT_STARTED;

   }

 

-  // If reset then clear the filter registers

-  if (Reset) {

-    Enable |= EFI_SIMPLE_NETWORK_RECEIVE_MULTICAST;

-    IndirectMACWrite32 (INDIRECT_MAC_INDEX_HASHL, 0x00000000);

-    IndirectMACWrite32 (INDIRECT_MAC_INDEX_HASHH, 0x00000000);

+  if ((Enable  & (~Mode->ReceiveFilterMask)) ||

+      (Disable & (~Mode->ReceiveFilterMask))    ) {

+    return EFI_INVALID_PARAMETER;

   }

 

-  // Set the hash tables

-  if ((NumMfilter > 0) && (!Reset)) {

-

-    // Read the Multicast High Hash Table

-    MultHashTableHigh = IndirectMACRead32 (INDIRECT_MAC_INDEX_HASHH);

+  //

+  // Check the validity of the multicast setting and compute the

+  // hash values of the multicast mac addresses to listen to.

+  //

 

-    // Read the Multicast Low Hash Table

-    MultHashTableLow = IndirectMACRead32 (INDIRECT_MAC_INDEX_HASHL);

+  MultHashTableHigh = 0;

+  MultHashTableLow  = 0;

+  if ((!ResetMCastFilter)                                     &&

+      ((Disable & EFI_SIMPLE_NETWORK_RECEIVE_MULTICAST) == 0) &&

+      ((Enable  & EFI_SIMPLE_NETWORK_RECEIVE_MULTICAST) != 0)    ) {

+    if ((MCastFilterCnt == 0)                        ||

+        (MCastFilterCnt > Mode->MaxMCastFilterCount) ||

+        (MCastFilter == NULL)                           ) {

+      return EFI_INVALID_PARAMETER;

+    }

+    //

+    // Check the validity of all multicast addresses before to change

+    // anything.

+    //

+    for (Count = 0; Count < MCastFilterCnt; Count++) {

+      if ((MCastFilter[Count].Addr[0] & 1) == 0) {

+        return EFI_INVALID_PARAMETER;

+      }

+    }

 

+    //

     // Go through each filter address and set appropriate bits on hash table

-    for (Count = 0; Count < NumMfilter; Count++) {

+    //

+    for (Count = 0; Count < MCastFilterCnt; Count++) {

+      Mac = &(MCastFilter[Count]);

+      CopyMem (&Mode->MCastFilter[Count], Mac, sizeof(EFI_MAC_ADDRESS));

 

-      // Generate a 32-bit CRC for Ethernet

-      Crc = GenEtherCrc32 (&Mfilter[Count],6);

+      Crc = GenEtherCrc32 (Mac, NET_ETHER_ADDR_LEN);

       //gBS->CalculateCrc32 ((VOID*)&Mfilter[Count],6,&Crc); <-- doesn't work as desired

 

-      // Get the most significant 6 bits to index hash registers

-      BitToSelect = (Crc >> 26) & 0x3F;

+      //

+      // The most significant 6 bits of the MAC address CRC constitute the hash

+      // value of the MAC address.

+      //

+      HashValue = (Crc >> 26) & 0x3F;

 

       // Select hashlow register if MSB is not set

-      if ((BitToSelect & 0x20) == 0) {

-        MultHashTableLow |= (1 << BitToSelect);

+      if ((HashValue & 0x20) == 0) {

+        MultHashTableLow |= (1 << HashValue);

       } else {

-        MultHashTableHigh |= (1 << (BitToSelect & 0x1F));

+        MultHashTableHigh |= (1 << (HashValue & 0x1F));

       }

     }

-

-    // Write the desired hash

-    IndirectMACWrite32 (INDIRECT_MAC_INDEX_HASHL, MultHashTableLow);

-    IndirectMACWrite32 (INDIRECT_MAC_INDEX_HASHH, MultHashTableHigh);

+    Mode->MCastFilterCount = MCastFilterCnt;

+  } else if (ResetMCastFilter) {

+    Mode->MCastFilterCount = 0;

+  } else {

+    MultHashTableLow  = IndirectMACRead32 (INDIRECT_MAC_INDEX_HASHL);

+    MultHashTableHigh = IndirectMACRead32 (INDIRECT_MAC_INDEX_HASHH);

   }

 

+  //

+  // Write the mask of the selected hash values for the multicast filtering.

+  // The two masks are set to zero if the multicast filtering is not enabled.

+  //

+  IndirectMACWrite32 (INDIRECT_MAC_INDEX_HASHL, MultHashTableLow);

+  IndirectMACWrite32 (INDIRECT_MAC_INDEX_HASHH, MultHashTableHigh);

+

   // Read MAC controller

   MacCSRValue = IndirectMACRead32 (INDIRECT_MAC_INDEX_CR);

 

@@ -632,25 +696,35 @@ SnpReceiveFilters (
   return EFI_SUCCESS;

 }

 

-/*

- *  UEFI StationAddress() function

- *

- */

+/**

+  Modify of reset the current station address

+

+  @param[in]  Snp               A pointer to the EFI_SIMPLE_NETWORK_PROTOCOL

+                                instance.

+  @param[in]  Reset             Flag used to reset the station address to the

+                                LAN9118's permanent address.

+  @param[in]  New               New station address to be used for the network interface.

+

+  @retval  EFI_SUCCESS            The LAN9118's station address was updated.

+  @retval  EFI_NOT_STARTED        The LAN9118 has not been started.

+  @retval  EFI_INVALID_PARAMETER  One or more of the following conditions is TRUE :

+                                  . The "New" station address is invalid.

+                                  . "Reset" is FALSE and "New" is NULL.

+  @retval  EFI_DEVICE_ERROR       The LAN9118 has been started but not initialized.

+

+**/

 EFI_STATUS

 EFIAPI

 SnpStationAddress (

-  IN        EFI_SIMPLE_NETWORK_PROTOCOL *Snp,

-  IN        BOOLEAN Reset,

-  IN        EFI_MAC_ADDRESS *NewMac

+  IN  EFI_SIMPLE_NETWORK_PROTOCOL  *Snp,

+  IN  BOOLEAN                      Reset,

+  IN  EFI_MAC_ADDRESS              *New

 )

 {

-  DEBUG ((DEBUG_NET, "SnpStationAddress()\n"));

-

   UINT32 Count;

-  UINT8  PermAddr[6];

-  UINT64 DefaultMacAddress;

+  UINT8  PermAddr[NET_ETHER_ADDR_LEN];

 

-  Count = 0;

+  DEBUG ((DEBUG_NET, "SnpStationAddress()\n"));

 

   // Check Snp instance

   if (Snp == NULL) {

@@ -670,27 +744,31 @@ SnpStationAddress (
   if (Reset) {

     // Try using EEPROM first. Read the first byte of data from EEPROM at the address 0x0

     if ((IndirectEEPROMRead32 (0) & 0xFF) == EEPROM_EXTERNAL_SERIAL_EEPROM) {

-      for (Count = 1; Count < 7; Count++) {

-        PermAddr[Count - 1] = IndirectEEPROMRead32 (Count);

+      for (Count = 0; Count < NET_ETHER_ADDR_LEN; Count++) {

+        PermAddr[Count] = IndirectEEPROMRead32 (Count + 1);

       }

-

-      // Write address

+      New = (EFI_MAC_ADDRESS *) PermAddr;

       Lan9118SetMacAddress ((EFI_MAC_ADDRESS *) PermAddr, Snp);

     } else {

       DEBUG ((EFI_D_ERROR, "Lan9118: Warning: No valid MAC address in EEPROM, using fallback\n"));

-      DefaultMacAddress = FixedPcdGet64 (PcdLan9118DefaultMacAddress);

-      Lan9118SetMacAddress ((EFI_MAC_ADDRESS *) &DefaultMacAddress, Snp);

+      New = (EFI_MAC_ADDRESS*) (FixedPcdGet64 (PcdLan9118DefaultMacAddress));

     }

   } else {

     // Otherwise use the specified new MAC address

-    if (NewMac == NULL) {

+    if (New == NULL) {

+      return EFI_INVALID_PARAMETER;

+    }

+    //

+    // If it is a multicast address, it is not valid.

+    //

+    if (New->Addr[0] & 0x01) {

       return EFI_INVALID_PARAMETER;

     }

-

-    // Write address

-    Lan9118SetMacAddress (NewMac, Snp);

   }

 

+  // Write address

+  Lan9118SetMacAddress (New, Snp);

+

   return EFI_SUCCESS;

 }

 

@@ -707,7 +785,8 @@ SnpStatistics (
       OUT   EFI_NETWORK_STATISTICS *Statistics

   )

 {

-  LAN9118_DRIVER *LanDriver;

+  LAN9118_DRIVER  *LanDriver;

+  EFI_STATUS      Status;

 

   LanDriver = INSTANCE_FROM_SNP_THIS (Snp);

 

@@ -727,31 +806,39 @@ SnpStatistics (
     return EFI_NOT_STARTED;

   }

 

-  // Check pointless condition

-  if ((!Reset) && (StatSize == NULL) && (Statistics == NULL)) {

-    return EFI_SUCCESS;

-  }

-

-  // Check the parameters

-  if ((StatSize == NULL) && (Statistics != NULL)) {

-    return EFI_INVALID_PARAMETER;

-  }

-

-  // Do a reset if required

+  //

+  // Do a reset if required. It is not clearly stated in the UEFI specification

+  // whether the reset has to be done before to copy the statistics in "Statictics"

+  // or after. It is a bit strange to do it before but that is what is expected by

+  // the SCT test on Statistics() with reset : "0x3de76704,0x4bf5,0x42cd,0x8c,0x89,

+  // 0x54,0x7e,0x4f,0xad,0x4f,0x24".

+  //

   if (Reset) {

     ZeroMem (&LanDriver->Stats, sizeof(EFI_NETWORK_STATISTICS));

   }

 

-  // Check buffer size

-  if (*StatSize < sizeof(EFI_NETWORK_STATISTICS)) {

-    *StatSize = sizeof(EFI_NETWORK_STATISTICS);

-    return EFI_BUFFER_TOO_SMALL;

+  Status = EFI_SUCCESS;

+  if (StatSize == NULL) {

+    if (Statistics != NULL) {

+      return EFI_INVALID_PARAMETER;

+    }

+  } else {

+    if (Statistics == NULL) {

+      Status = EFI_BUFFER_TOO_SMALL;

+    } else {

+      // Fill in the statistics

+      CopyMem (

+        Statistics, &LanDriver->Stats,

+        MIN (*StatSize, sizeof (EFI_NETWORK_STATISTICS))

+        );

+      if (*StatSize < sizeof (EFI_NETWORK_STATISTICS)) {

+        Status = EFI_BUFFER_TOO_SMALL;

+      }

+    }

+    *StatSize = sizeof (EFI_NETWORK_STATISTICS);

   }

 

-  // Fill in the statistics

-  CopyMem (Statistics, &LanDriver->Stats, sizeof(EFI_NETWORK_STATISTICS));

-

-  return EFI_SUCCESS;

+  return Status;

 }

 

 /*

@@ -846,9 +933,9 @@ SnpNvData (
 EFI_STATUS

 EFIAPI

 SnpGetStatus (

-  IN        EFI_SIMPLE_NETWORK_PROTOCOL* Snp,

-      OUT   UINT32 *IrqStat  OPTIONAL,

-      OUT   VOID **TxBuff    OPTIONAL

+  IN   EFI_SIMPLE_NETWORK_PROTOCOL  *Snp,

+  OUT  UINT32                       *IrqStat  OPTIONAL,

+  OUT  VOID                         **TxBuff  OPTIONAL

   )

 {

   UINT32          FifoInt;

@@ -866,7 +953,12 @@ SnpGetStatus (
     return EFI_INVALID_PARAMETER;

   }

 

-  if (Snp->Mode->State != EfiSimpleNetworkInitialized) {

+  // Check that driver was started and initialised

+  if (Snp->Mode->State == EfiSimpleNetworkStarted) {

+    DEBUG ((EFI_D_WARN, "Warning: LAN9118 Driver not initialized\n"));

+    return EFI_DEVICE_ERROR;

+  } else if (Snp->Mode->State == EfiSimpleNetworkStopped) {

+    DEBUG ((EFI_D_WARN, "Warning: LAN9118 Driver in stopped state\n"));

     return EFI_NOT_STARTED;

   }

 

@@ -1018,7 +1110,13 @@ SnpTransmit (
   if ((Snp == NULL) || (Data == NULL)) {

     return EFI_INVALID_PARAMETER;

   }

-  if (Snp->Mode->State != EfiSimpleNetworkInitialized) {

+

+  // Check that driver was started and initialised

+  if (Snp->Mode->State == EfiSimpleNetworkStarted) {

+    DEBUG ((EFI_D_WARN, "Warning: LAN9118 Driver not initialized\n"));

+    return EFI_DEVICE_ERROR;

+  } else if (Snp->Mode->State == EfiSimpleNetworkStopped) {

+    DEBUG ((EFI_D_WARN, "Warning: LAN9118 Driver in stopped state\n"));

     return EFI_NOT_STARTED;

   }

 

@@ -1033,6 +1131,13 @@ SnpTransmit (
     }

   }

 

+  //

+  // Check validity of BufferSize

+  //

+  if (BuffSize < Snp->Mode->MediaHeaderSize) {

+      return EFI_BUFFER_TOO_SMALL;

+  }

+

   // Before transmitting check the link status

   /*if (CheckLinkStatus (0, Snp) < 0) {

     return EFI_NOT_READY;

@@ -1193,11 +1298,16 @@ SnpReceive (
 #endif

 

   // Check preliminaries

-  if ((Snp == NULL) || (Data == NULL)) {

+  if ((Snp == NULL) || (Data == NULL) || (BuffSize == NULL)) {

     return EFI_INVALID_PARAMETER;

   }

 

-  if (Snp->Mode->State != EfiSimpleNetworkInitialized) {

+  // Check that driver was started and initialised

+  if (Snp->Mode->State == EfiSimpleNetworkStarted) {

+    DEBUG ((EFI_D_WARN, "Warning: LAN9118 Driver not initialized\n"));

+    return EFI_DEVICE_ERROR;

+  } else if (Snp->Mode->State == EfiSimpleNetworkStopped) {

+    DEBUG ((EFI_D_WARN, "Warning: LAN9118 Driver in stopped state\n"));

     return EFI_NOT_STARTED;

   }