Re: [Linaro-uefi] [PATCH v2 2/2] OpenPlatformPkg/Lan91xDxe: Fixed TX recycle buffer overflow

Leif, ping!

On 16 June 2016 at 09:59, Ryan Harkin ryan.harkin@linaro.org wrote:

On 14 June 2016 at 23:38, Daniil Egranov daniil.egranov@arm.com wrote:

...

The patch is fixing the TX recycle buffer overflow.

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Daniil Egranov daniil.egranov@arm.com

Reviewed-by: Ryan Harkin ryan.harkin@linaro.org Tested-by: Ryan Harkin ryan.harkin@linaro.org

Tested on FVP Base AEMv8 and Foundation models.

...

---

Drivers/Net/Lan91xDxe/Lan91xDxe.c | 100 ++++++++++++++++++-------------------- 1 file changed, 48 insertions(+), 52 deletions(-)

diff --git a/Drivers/Net/Lan91xDxe/Lan91xDxe.c b/Drivers/Net/Lan91xDxe/Lan91xDxe.c index 053f081..76d1233 100644 --- a/Drivers/Net/Lan91xDxe/Lan91xDxe.c +++ b/Drivers/Net/Lan91xDxe/Lan91xDxe.c @@ -65,10 +65,8 @@ typedef struct _LAN91X_DRIVER { EFI_NETWORK_STATISTICS Stats;

// Transmit Buffer recycle queue -#define TX_QUEUE_DEPTH 16

• VOID *TxQueue[TX_QUEUE_DEPTH];
• UINTN TxQueHead;
• UINTN TxQueTail;

• LIST_ENTRY TransmitQueueHead;

  // Register access variables UINTN IoBase; // I/O Base Address

@@ -137,47 +135,20 @@ STATIC CHAR16 CONST * CONST ChipIds[ 16 ] = { NULL, NULL, NULL };

+/* ------------------ TxBuffer Queue structures ------------------- */

-/* ------------------ TxBuffer Queue functions ------------------- */

-#define TxQueNext(off) ((((off) + 1) >= TX_QUEUE_DEPTH) ? 0 : ((off) + 1))

-STATIC -BOOLEAN -TxQueInsert (

• IN LAN91X_DRIVER *LanDriver,
• IN VOID *Buffer
• )

-{

• if (TxQueNext (LanDriver->TxQueTail) == LanDriver->TxQueHead) {
• return FALSE;
• }
• LanDriver->TxQueue[LanDriver->TxQueTail] = Buffer;
• LanDriver->TxQueTail = TxQueNext (LanDriver->TxQueTail);
• return TRUE;

-}

-STATIC -VOID -*TxQueRemove (

• IN LAN91X_DRIVER *LanDriver
• )

-{

• VOID *Buffer;
• if (LanDriver->TxQueTail == LanDriver->TxQueHead) {
• return NULL;
• }

+typedef struct {

• VOID *Buf;
• UINTN Length;

+} MSK_SYSTEM_BUF;

• Buffer = LanDriver->TxQueue[LanDriver->TxQueHead];
• LanDriver->TxQueue[LanDriver->TxQueHead] = NULL;
• LanDriver->TxQueHead = TxQueNext (LanDriver->TxQueHead);

+typedef struct {

• UINTN Signature;
• LIST_ENTRY Link;
• MSK_SYSTEM_BUF SystemBuf;

+} MSK_LINKED_SYSTEM_BUF;

• return Buffer;

-} +#define TX_MBUF_SIGNATURE SIGNATURE_32 ('t','x','m','b')

/* ------------------ MAC Address Hash Calculations ------------------- */

@@ -1643,11 +1614,12 @@ SnpGetStatus ( OUT VOID **TxBuff OPTIONAL ) {

• LAN91X_DRIVER *LanDriver;
• EFI_TPL SavedTpl;
• EFI_STATUS Status;
• BOOLEAN MediaPresent;
• UINT8 IstReg;

• LAN91X_DRIVER *LanDriver;

• EFI_TPL SavedTpl;

• EFI_STATUS Status;

• BOOLEAN MediaPresent;

• UINT8 IstReg;

• MSK_LINKED_SYSTEM_BUF *LinkedTXRecycleBuff;

  // Check preliminaries if (Snp == NULL) {

@@ -1689,8 +1661,18 @@ SnpGetStatus ( }

// Pass back the completed buffer address

• // The transmit buffer status is not read when TxBuf is NULL if (TxBuff != NULL) {

• *TxBuff = TxQueRemove (LanDriver);

• *((UINT8 **) TxBuff) = (UINT8 *) 0;

• if( !IsListEmpty (&LanDriver->TransmitQueueHead))

• {

•  LinkedTXRecycleBuff = CR (GetFirstNode (&LanDriver->TransmitQueueHead), MSK_LINKED_SYSTEM_BUF, Link, TX_MBUF_SIGNATURE);
  

•  if(LinkedTXRecycleBuff != NULL) {
  

•    *TxBuff = LinkedTXRecycleBuff->SystemBuf.Buf;
  

•    RemoveEntryList (&LinkedTXRecycleBuff->Link);
  

•    FreePool (LinkedTXRecycleBuff);
  

•  }
  

• } }

  // Update the media status

@@ -1733,6 +1715,8 @@ SnpTransmit ( UINTN Retries; UINT16 Proto; UINT8 PktNum;

• MSK_LINKED_SYSTEM_BUF *LinkedTXRecycleBuff;

• // Check preliminaries if ((Snp == NULL) || (BufAddr == NULL)) {

@@ -1883,14 +1867,23 @@ SnpTransmit ( ReturnUnlock (EFI_DEVICE_ERROR); }

• // Update the Rx statistics

• // Update the Tx statistics LanDriver->Stats.TxTotalBytes += BufSize; LanDriver->Stats.TxGoodFrames += 1;

  // Update the Tx Buffer cache

• if (!TxQueInsert (LanDriver, BufAddr)) {
• DEBUG((EFI_D_WARN, "LAN91x: SnpTransmit(): TxQueue insert failure.\n"));
• }

• LinkedTXRecycleBuff = AllocateZeroPool (sizeof (MSK_LINKED_SYSTEM_BUF));

• if (LinkedTXRecycleBuff == NULL) {

• return EFI_OUT_OF_RESOURCES;

• }

• LinkedTXRecycleBuff->Signature = TX_MBUF_SIGNATURE;

• //

• // Add the passed Buffer to the transmit queue. Don't copy.

• //

• LinkedTXRecycleBuff->SystemBuf.Buf = BufAddr;

• LinkedTXRecycleBuff->SystemBuf.Length = BufSize;

• InsertTailList (&LanDriver->TransmitQueueHead, &LinkedTXRecycleBuff->Link);

• Status = EFI_SUCCESS;

  // Dump the packet header

@@ -2161,6 +2154,9 @@ Lan91xDxeEntry ( PrintPhyRegisters (LanDriver); #endif

• // Initialize transmit queue
• InitializeListHead (&LanDriver->TransmitQueueHead);
• // Assign fields and func pointers Snp->Revision = EFI_SIMPLE_NETWORK_PROTOCOL_REVISION; Snp->WaitForPacket = NULL;

-- 2.7.4

---

Linaro-uefi mailing list Linaro-uefi@lists.linaro.org https://lists.linaro.org/mailman/listinfo/linaro-uefi