From: Laszlo Ersek <lersek@redhat.com>
Date: Mon, 26 Aug 2013 02:47:41 +0000 (+0000)
Subject: MdeModulePkg/DiskIoDxe: fix source/destination pointer of overrun transfer
X-Git-Tag: edk2-stable201903~12304
X-Git-Url: https://git.proxmox.com/?p=mirror_edk2.git;a=commitdiff_plain;h=4e39b75e7ec56eaf71a1bab2d8c3bd487c7e2273

MdeModulePkg/DiskIoDxe: fix source/destination pointer of overrun transfer

DiskIoCreateSubtaskList() may split the transfer into three segments:
- a leading segment, called underrun, which is the fractional, trailing
  subset of the first underlying block,
- a middle segment, which is an integral multiple of underlying blocks,
- a trailing segment, called overrun, which is the fractional, leading
  subset of the last underlying block.

This is an example read from the /EFI/BOOT/BOOTX64.EFI file, on the
RHEL-6.4 installation ISO (debug log enabled with EFI_D_BLKIO). The
underlying block size is 2048 bytes (IDE CD-ROM).

DiskIo: Create subtasks for task: Offset/BufferSize/Buffer = 0000000000004600/00002000/BD890018
  R:Lba/Offset/Length/WorkingBuffer/Buffer = 0000000000000008/00000600/00000200/BD90D000/BD890018
  R:Lba/Offset/Length/WorkingBuffer/Buffer = 000000000000000C/00000000/00000600/BD90D000/BD890218
  R:Lba/Offset/Length/WorkingBuffer/Buffer = 0000000000000009/00000000/00001800/00000000/BD890218

The first line corresponds to the underrun.
The second line corresponds to the overrun.
The third line corresponds to the middle segment.

In decimal:
- task: read 8192 bytes from offset 17920, storing it at BD890018
- underrun:
    - read block 8 [16384..18432) into the transfer area,
    - copy 512 bytes from offset 1536 of the transfer area to BD890018
      (target buffer offset 0, running total: 512)
- middle segment:
    - read blocks 9, 10, 11 [18432..24576) into the transfer area,
    - copy 6144 bytes from offset 0 of the transfer area to BD890218
      (target buffer offset 512, running total: 6656)
- overrun:
    - read block 12 [24576..26624) into the transfer area,
    - copy 1536 bytes from offset 0 of the transfer area to BD890218 (!!!)
      (target buffer offset 512 (!!!), running total 8192)

The values marked with (!!!) constitute the bug --
DiskIoCreateSubtaskList() doesn't take the size of the middle segment into
account when it calculates the destination (for reads) or source (for
writes) pointer for the overrun. This leads to data corruption.

When reading, data is copied form the transfer area to the target buffer
with

  CopyMem (Subtask->Buffer, Subtask->WorkingBuffer + Subtask->Offset, Subtask->Length);

calls in DiskIo2OnReadWriteComplete() for nonblocking reads, and in
DiskIo2ReadWriteDisk() for blocking reads. Therefore it's enough to adjust
Subtask->Buffer when it is initialized. (See BD891A18 below.)

DiskIo: Create subtasks for task: Offset/BufferSize/Buffer = 0000000000004600/00002000/BD890018
  R:Lba/Offset/Length/WorkingBuffer/Buffer = 0000000000000008/00000600/00000200/BD90D000/BD890018
  R:Lba/Offset/Length/WorkingBuffer/Buffer = 000000000000000C/00000000/00000600/BD90D000/BD891A18
  R:Lba/Offset/Length/WorkingBuffer/Buffer = 0000000000000009/00000000/00001800/00000000/BD890218

The patched call to DiskIoCreateSubtask() is also executed for write
requests. The changed Subtask->Buffer initialization fixes the "overrun
half writes" in DiskIo2ReadWriteDisk() too:

  //
  // A sub task before this one should be a block read operation, causing
  // the WorkingBuffer filled with the entire one block data.
  //
  CopyMem (Subtask->WorkingBuffer + Subtask->Offset, Subtask->Buffer, Subtask->Length);

This code doubles for underrun and overrun half-writes. The patch doesn't
modify the underrun case.

If we're storing the overrun at the beginning of the pre-read last block
(which we're going to write out as a full block), then
- Subtask->Offset == 0,
- Subtask->Length == OverRun,
- the first byte *not* accessed in the source area is
  ((Buffer + UnderRunLength) + BufferSize) + OverRun.

Contributed-under: TianoCore Contribution Agreement 1.0

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>

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

diff --git a/MdeModulePkg/Universal/Disk/DiskIoDxe/DiskIo.c b/MdeModulePkg/Universal/Disk/DiskIoDxe/DiskIo.c
index e8af46f6c0..ba61300911 100644
--- a/MdeModulePkg/Universal/Disk/DiskIoDxe/DiskIo.c
+++ b/MdeModulePkg/Universal/Disk/DiskIoDxe/DiskIo.c
@@ -642,7 +642,7 @@ DiskIoCreateSubtaskList (
       InsertTailList (Subtasks, &Subtask->Link);
     }
 
-    Subtask = DiskIoCreateSubtask (Write, OverRunLba, 0, OverRun, WorkingBuffer, BufferPtr, Blocking);
+    Subtask = DiskIoCreateSubtask (Write, OverRunLba, 0, OverRun, WorkingBuffer, BufferPtr + BufferSize, Blocking);
     if (Subtask == NULL) {
       goto Done;
     }