From 9202304c180e811ab7b64ccf01fe11187969fe9b Mon Sep 17 00:00:00 2001 From: Jiaxin Wu Date: Thu, 25 Oct 2018 15:31:10 +0800 Subject: [PATCH] MdeModulePke/Mtftp4Dxe: Correct the total received and saved block number. The block returned from Mtftp4RemoveBlockNum is not the total received and saved block number if it works in passive (Slave) mode. The issue was exposed by the EMS test. Cc: Ye Ting Cc: Fu Siyuan Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Wu Jiaxin Reviewed-by: Ye Ting --- .../Universal/Network/Mtftp4Dxe/Mtftp4Impl.h | 6 +++++- .../Universal/Network/Mtftp4Dxe/Mtftp4Rrq.c | 16 +++++++++++----- .../Universal/Network/Mtftp4Dxe/Mtftp4Support.c | 10 +++++----- .../Universal/Network/Mtftp4Dxe/Mtftp4Support.h | 6 +++--- .../Universal/Network/Mtftp4Dxe/Mtftp4Wrq.c | 6 +++--- 5 files changed, 27 insertions(+), 17 deletions(-) diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.h b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.h index de304f4e70..be2f8af6e4 100644 --- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.h +++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.h @@ -126,9 +126,13 @@ struct _MTFTP4_PROTOCOL { UINT16 WindowSize; // - // Record the total received block number and the already acked block number. + // Record the total received and saved block number. // UINT64 TotalBlock; + + // + // Record the acked block number. + // UINT64 AckedBlock; // diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Rrq.c b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Rrq.c index fedf1cde46..6960e322a5 100644 --- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Rrq.c +++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Rrq.c @@ -154,6 +154,7 @@ Mtftp4RrqSaveBlock ( UINT16 Block; UINT64 Start; UINT32 DataLen; + UINT64 BlockCounter; BOOLEAN Completed; Completed = FALSE; @@ -174,10 +175,10 @@ Mtftp4RrqSaveBlock ( // Remove this block number from the file hole. If Mtftp4RemoveBlockNum // returns EFI_NOT_FOUND, the block has been saved, don't save it again. // Note that : For bigger files, allowing the block counter to roll over - // to accept transfers of unlimited size. So TotalBlock is memorised as + // to accept transfers of unlimited size. So BlockCounter is memorised as // continuous block counter. // - Status = Mtftp4RemoveBlockNum (&Instance->Blocks, Block, Completed, &Instance->TotalBlock); + Status = Mtftp4RemoveBlockNum (&Instance->Blocks, Block, Completed, &BlockCounter); if (Status == EFI_NOT_FOUND) { return EFI_SUCCESS; @@ -200,7 +201,7 @@ Mtftp4RrqSaveBlock ( } if (Token->Buffer != NULL) { - Start = MultU64x32 (Instance->TotalBlock - 1, Instance->BlkSize); + Start = MultU64x32 (BlockCounter - 1, Instance->BlkSize); if (Start + DataLen <= Token->BufferSize) { CopyMem ((UINT8 *) Token->Buffer + Start, Packet->Data.Data, DataLen); @@ -271,9 +272,9 @@ Mtftp4RrqHandleData ( ASSERT (Expected >= 0); // - // If we are active and received an unexpected packet, transmit + // If we are active (Master) and received an unexpected packet, transmit // the ACK for the block we received, then restart receiving the - // expected one. If we are passive, save the block. + // expected one. If we are passive (Slave), save the block. // if (Instance->Master && (Expected != BlockNum)) { // @@ -288,6 +289,11 @@ Mtftp4RrqHandleData ( return Status; } + // + // Record the total received and saved block number. + // + Instance->TotalBlock ++; + // // Reset the passive client's timer whenever it received a // valid data packet. diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c index 71fd979b3a..5e282e9c4b 100644 --- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c +++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c @@ -158,8 +158,8 @@ Mtftp4SetLastBlockNum ( @param Head The block range list to remove from @param Num The block number to remove - @param Completed Whether Num is the last block number - @param TotalBlock The continuous block number in all + @param Completed Whether Num is the last block number. + @param BlockCounter The continuous block counter instead of the value after roll-over. @retval EFI_NOT_FOUND The block number isn't in the block range list @retval EFI_SUCCESS The block number has been removed from the list @@ -171,7 +171,7 @@ Mtftp4RemoveBlockNum ( IN LIST_ENTRY *Head, IN UINT16 Num, IN BOOLEAN Completed, - OUT UINT64 *TotalBlock + OUT UINT64 *BlockCounter ) { MTFTP4_BLOCK_RANGE *Range; @@ -220,10 +220,10 @@ Mtftp4RemoveBlockNum ( // wrap to zero, because this is the simplest to implement. Here we choose // this solution. // - *TotalBlock = Num; + *BlockCounter = Num; if (Range->Round > 0) { - *TotalBlock += Range->Bound + MultU64x32 ((UINTN) (Range->Round -1), (UINT32) (Range->Bound + 1)) + 1; + *BlockCounter += Range->Bound + MultU64x32 ((UINTN) (Range->Round -1), (UINT32) (Range->Bound + 1)) + 1; } if (Range->Start > Range->Bound) { diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.h b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.h index 6cc2756bc8..f7a6755fe8 100644 --- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.h +++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.h @@ -92,8 +92,8 @@ Mtftp4SetLastBlockNum ( @param Head The block range list to remove from @param Num The block number to remove - @param Completed Wether Num is the last block number - @param TotalBlock The continuous block number in all + @param Completed Whether Num is the last block number. + @param BlockCounter The continuous block counter instead of the value after roll-over. @retval EFI_NOT_FOUND The block number isn't in the block range list @retval EFI_SUCCESS The block number has been removed from the list @@ -105,7 +105,7 @@ Mtftp4RemoveBlockNum ( IN LIST_ENTRY *Head, IN UINT16 Num, IN BOOLEAN Completed, - OUT UINT64 *TotalBlock + OUT UINT64 *BlockCounter ); /** diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Wrq.c b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Wrq.c index ea309e2d6b..ee70accbcd 100644 --- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Wrq.c +++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Wrq.c @@ -149,7 +149,7 @@ Mtftp4WrqHandleAck ( { UINT16 AckNum; INTN Expected; - UINT64 TotalBlock; + UINT64 BlockCounter; *Completed = FALSE; AckNum = NTOHS (Packet->Ack.Block[0]); @@ -168,9 +168,9 @@ Mtftp4WrqHandleAck ( // // Remove the acked block number, if this is the last block number, // tell the Mtftp4WrqInput to finish the transfer. This is the last - // block number if the block range are empty.. + // block number if the block range are empty. // - Mtftp4RemoveBlockNum (&Instance->Blocks, AckNum, *Completed,&TotalBlock); + Mtftp4RemoveBlockNum (&Instance->Blocks, AckNum, *Completed, &BlockCounter); Expected = Mtftp4GetNextBlockNum (&Instance->Blocks);