Re: [PATCH v3 2/2] MdeModulePkg/RamDiskDxe: Restrict on RAM disk size (CVE-2018-12180)

["Ni, Ray" <ray.ni@intel.com>]

Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: Wu, Hao A <hao.a.wu@intel.com>
> Sent: Tuesday, February 26, 2019 8:57 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> Ni, Ray <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Laszlo Ersek
> <lersek@redhat.com>
> Subject: [PATCH v3 2/2] MdeModulePkg/RamDiskDxe: Restrict on RAM disk
> size (CVE-2018-12180)
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1134
> 
> Originally, the block size of created Ram disks is hard-coded to 512 bytes.
> However, if the total size of the Ram disk is not a multiple of 512 bytes, there
> will be potential memory access issues when dealing with the last block of
> the Ram disk.
> 
> This commit will adjust the block size of the Ram disks to ensure that the total
> size is a multiple of the block size.
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> ---
>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h     |  6 +++---
>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskBlockIo.c  | 20
> ++++++++++++++------
> MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c |  5 +++--
>  3 files changed, 20 insertions(+), 11 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h
> b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h
> index 08a8ca94c9..72f2bfe179 100644
> --- a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h
> +++ b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h
> @@ -1,7 +1,7 @@
>  /** @file
>    The header file of RamDiskDxe driver.
> 
> -  Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2016 - 2019, Intel Corporation. All rights
> + reserved.<BR>
>    This program and the accompanying materials
>    are licensed and made available under the terms and conditions of the BSD
> License
>    which accompanies this distribution.  The full text of the license may be
> found at @@ -49,9 +49,9 @@  ///
> 
>  //
> -// Block size for RAM disk
> +// Default block size for RAM disk
>  //
> -#define RAM_DISK_BLOCK_SIZE 512
> +#define RAM_DISK_DEFAULT_BLOCK_SIZE 512
> 
>  //
>  // Iterate through the double linked list. NOT delete safe diff --git
> a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskBlockIo.c
> b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskBlockIo.c
> index 4f74b5ef15..8926ad7d2f 100644
> --- a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskBlockIo.c
> +++ b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskBlockIo.c
> @@ -1,7 +1,7 @@
>  /** @file
>    Produce EFI_BLOCK_IO_PROTOCOL on a RAM disk device.
> 
> -  Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2016 - 2019, Intel Corporation. All rights
> + reserved.<BR>
>    This program and the accompanying materials
>    are licensed and made available under the terms and conditions of the BSD
> License
>    which accompanies this distribution.  The full text of the license may be
> found at @@ -54,6 +54,7 @@ RamDiskInitBlockIo (
>    EFI_BLOCK_IO_PROTOCOL           *BlockIo;
>    EFI_BLOCK_IO2_PROTOCOL          *BlockIo2;
>    EFI_BLOCK_IO_MEDIA              *Media;
> +  UINT32                          Remainder;
> 
>    BlockIo  = &PrivateData->BlockIo;
>    BlockIo2 = &PrivateData->BlockIo2;
> @@ -69,11 +70,18 @@ RamDiskInitBlockIo (
>    Media->LogicalPartition = FALSE;
>    Media->ReadOnly         = FALSE;
>    Media->WriteCaching     = FALSE;
> -  Media->BlockSize        = RAM_DISK_BLOCK_SIZE;
> -  Media->LastBlock        = DivU64x32 (
> -                              PrivateData->Size + RAM_DISK_BLOCK_SIZE - 1,
> -                              RAM_DISK_BLOCK_SIZE
> -                              ) - 1;
> +
> +  for (Media->BlockSize = RAM_DISK_DEFAULT_BLOCK_SIZE;
> +       Media->BlockSize >= 1;
> +       Media->BlockSize = Media->BlockSize >> 1) {
> +    Media->LastBlock = DivU64x32Remainder (PrivateData->Size, Media-
> >BlockSize, &Remainder) - 1;
> +    if (Remainder == 0) {
> +      break;
> +    }
> +  }
> +  ASSERT (Media->BlockSize != 0);
> +
> +  return;
>  }
> 
> 
> diff --git a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c
> b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c
> index 6784e2b2f1..e8250d5c1b 100644
> --- a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c
> +++ b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c
> @@ -1,7 +1,7 @@
>  /** @file
>    The realization of EFI_RAM_DISK_PROTOCOL.
> 
> -  Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2016 - 2019, Intel Corporation. All rights
> + reserved.<BR>
>    (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
>    This program and the accompanying materials
>    are licensed and made available under the terms and conditions of the BSD
> License @@ -613,7 +613,8 @@ RamDiskRegister (
>    //
>    // Add check to prevent data read across the memory boundary
>    //
> -  if (RamDiskBase + RamDiskSize > ((UINTN) -1) - RAM_DISK_BLOCK_SIZE + 1)
> {
> +  if ((RamDiskSize > MAX_UINTN) ||
> +      (RamDiskBase > MAX_UINTN - RamDiskSize + 1)) {
>      return EFI_INVALID_PARAMETER;
>    }
> 
> --
> 2.12.0.windows.1