[PATCH v2 0/2] MdeModulePkg: Resolve buffer cross boundary access in Ramdisk

[PATCH v2 0/2] MdeModulePkg: Resolve buffer cross boundary access in Ramdisk

[Hao Wu]

V2 changes:

Correct CC list information.


V1 history:

The series will resolve a buffer cross boundary access issue during the
use of RAM disks. It is the mitigation for issue CVE-2018-12180.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>

Hao Wu (2):
  MdeModulePkg/PartitionDxe: Ensure blocksize can hold MBR (CVE FIX)
  MdeModulePkg/RamDiskDxe: Ramdisk size be multiple of BlkSize (CVE FIX)

 MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h     |  6 +++---
 MdeModulePkg/Universal/Disk/PartitionDxe/Gpt.c           |  9 ++++++++-
 MdeModulePkg/Universal/Disk/PartitionDxe/Mbr.c           |  9 ++++++++-
 MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskBlockIo.c  | 20 ++++++++++++++------
 MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c |  5 +++--
 5 files changed, 36 insertions(+), 13 deletions(-)

-- 
2.12.0.windows.1

[PATCH v2 1/2] MdeModulePkg/PartitionDxe: Ensure blocksize can hold MBR (CVE FIX)

[Hao Wu]

Fix CVE-2018-12180
https://bugzilla.tianocore.org/show_bug.cgi?id=1134

The commit adds checks for detecting GPT and MBR partitions.

These checks will ensure that the device block size is big enough to hold
an MBR (512 bytes).

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 MdeModulePkg/Universal/Disk/PartitionDxe/Gpt.c | 9 ++++++++-
 MdeModulePkg/Universal/Disk/PartitionDxe/Mbr.c | 9 ++++++++-
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Gpt.c b/MdeModulePkg/Universal/Disk/PartitionDxe/Gpt.c
index fe87761bde..d679cc208b 100644
--- a/MdeModulePkg/Universal/Disk/PartitionDxe/Gpt.c
+++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Gpt.c
@@ -14,7 +14,7 @@
   partition content and validate the GPT table and GPT entry.
 
 Copyright (c) 2018 Qualcomm Datacenter Technologies, Inc.
-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 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
@@ -237,6 +237,13 @@ PartitionInstallGptChildHandles (
   GptValidStatus = EFI_NOT_FOUND;
 
   //
+  // Ensure the block size can hold the MBR
+  //
+  if (BlockSize < sizeof (MASTER_BOOT_RECORD)) {
+    return EFI_NOT_FOUND;
+  }
+
+  //
   // Allocate a buffer for the Protective MBR
   //
   ProtectiveMbr = AllocatePool (BlockSize);
diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Mbr.c b/MdeModulePkg/Universal/Disk/PartitionDxe/Mbr.c
index b1a99ee85b..419f8a17a7 100644
--- a/MdeModulePkg/Universal/Disk/PartitionDxe/Mbr.c
+++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Mbr.c
@@ -13,7 +13,7 @@
 
 Copyright (c) 2018 Qualcomm Datacenter Technologies, Inc.
 Copyright (c) 2014, Hewlett-Packard Development Company, L.P.<BR>
-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 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
@@ -150,6 +150,13 @@ PartitionInstallMbrChildHandles (
   MediaId   = BlockIo->Media->MediaId;
   LastBlock = BlockIo->Media->LastBlock;
 
+  //
+  // Ensure the block size can hold the MBR
+  //
+  if (BlockSize < sizeof (MASTER_BOOT_RECORD)) {
+    return EFI_NOT_FOUND;
+  }
+
   Mbr = AllocatePool (BlockSize);
   if (Mbr == NULL) {
     return Found;
-- 
2.12.0.windows.1

[PATCH v2 2/2] MdeModulePkg/RamDiskDxe: Ramdisk size be multiple of BlkSize (CVE FIX)

[Hao Wu]

Fix CVE-2018-12180
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>
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

Re: [PATCH v2 0/2] MdeModulePkg: Resolve buffer cross boundary access in Ramdisk

[Laszlo Ersek]

On 02/26/19 08:45, Hao Wu wrote:
> V2 changes:
> 
> Correct CC list information.
> 
> 
> V1 history:
> 
> The series will resolve a buffer cross boundary access issue during the
> use of RAM disks. It is the mitigation for issue CVE-2018-12180.
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> 
> Hao Wu (2):
>   MdeModulePkg/PartitionDxe: Ensure blocksize can hold MBR (CVE FIX)
>   MdeModulePkg/RamDiskDxe: Ramdisk size be multiple of BlkSize (CVE FIX)
> 
>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h     |  6 +++---
>  MdeModulePkg/Universal/Disk/PartitionDxe/Gpt.c           |  9 ++++++++-
>  MdeModulePkg/Universal/Disk/PartitionDxe/Mbr.c           |  9 ++++++++-
>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskBlockIo.c  | 20 ++++++++++++++------
>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c |  5 +++--
>  5 files changed, 36 insertions(+), 13 deletions(-)
> 

Please put the exact CVE numbers in the subject lines.

Thanks
Laszlo

Re: [PATCH v2 0/2] MdeModulePkg: Resolve buffer cross boundary access in Ramdisk

[Wu, Hao A]

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Laszlo Ersek
> Sent: Tuesday, February 26, 2019 7:45 PM
> To: Wu, Hao A; edk2-devel@lists.01.org
> Cc: Zeng, Star
> Subject: Re: [edk2] [PATCH v2 0/2] MdeModulePkg: Resolve buffer cross
> boundary access in Ramdisk
> 
> On 02/26/19 08:45, Hao Wu wrote:
> > V2 changes:
> >
> > Correct CC list information.
> >
> >
> > V1 history:
> >
> > The series will resolve a buffer cross boundary access issue during the
> > use of RAM disks. It is the mitigation for issue CVE-2018-12180.
> >
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> >
> > Hao Wu (2):
> >   MdeModulePkg/PartitionDxe: Ensure blocksize can hold MBR (CVE FIX)
> >   MdeModulePkg/RamDiskDxe: Ramdisk size be multiple of BlkSize (CVE
> FIX)
> >
> >  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h     |  6 +++---
> >  MdeModulePkg/Universal/Disk/PartitionDxe/Gpt.c           |  9 ++++++++-
> >  MdeModulePkg/Universal/Disk/PartitionDxe/Mbr.c           |  9 ++++++++-
> >  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskBlockIo.c  | 20
> ++++++++++++++------
> >  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c |  5 +++--
> >  5 files changed, 36 insertions(+), 13 deletions(-)
> >
> 
> Please put the exact CVE numbers in the subject lines.

Thanks.
V3 series proposed.

Best Regards,
Hao Wu

> 
> Thanks
> Laszlo
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH v2 0/2] MdeModulePkg: Resolve buffer cross boundary access in Ramdisk

[Wu, Hao A]

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Laszlo Ersek
> Sent: Tuesday, February 26, 2019 7:45 PM
> To: Wu, Hao A; edk2-devel@lists.01.org
> Cc: Zeng, Star
> Subject: Re: [edk2] [PATCH v2 0/2] MdeModulePkg: Resolve buffer cross
> boundary access in Ramdisk
> 
> On 02/26/19 08:45, Hao Wu wrote:
> > V2 changes:
> >
> > Correct CC list information.
> >
> >
> > V1 history:
> >
> > The series will resolve a buffer cross boundary access issue during the
> > use of RAM disks. It is the mitigation for issue CVE-2018-12180.
> >
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> >
> > Hao Wu (2):
> >   MdeModulePkg/PartitionDxe: Ensure blocksize can hold MBR (CVE FIX)
> >   MdeModulePkg/RamDiskDxe: Ramdisk size be multiple of BlkSize (CVE
> FIX)
> >
> >  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h     |  6 +++---
> >  MdeModulePkg/Universal/Disk/PartitionDxe/Gpt.c           |  9 ++++++++-
> >  MdeModulePkg/Universal/Disk/PartitionDxe/Mbr.c           |  9 ++++++++-
> >  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskBlockIo.c  | 20
> ++++++++++++++------
> >  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c |  5 +++--
> >  5 files changed, 36 insertions(+), 13 deletions(-)
> >
> 
> Please put the exact CVE numbers in the subject lines.

Hello Laszlo and Liming,

I totally agree the commit subject line should include the CVE number.
But I have one feedback that, if the commit is for a CVE fix, is it
possible to exempt the commit subject from 71 characters limit?

I found it can be hard to summary the commit with the Package/Module plus
the CVE number information.

Best Regards,
Hao Wu

> 
> Thanks
> Laszlo
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH v2 0/2] MdeModulePkg: Resolve buffer cross boundary access in Ramdisk

[Laszlo Ersek]

On 02/27/19 07:56, Wu, Hao A wrote:
>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
>> Laszlo Ersek
>> Sent: Tuesday, February 26, 2019 7:45 PM
>> To: Wu, Hao A; edk2-devel@lists.01.org
>> Cc: Zeng, Star
>> Subject: Re: [edk2] [PATCH v2 0/2] MdeModulePkg: Resolve buffer cross
>> boundary access in Ramdisk
>>
>> On 02/26/19 08:45, Hao Wu wrote:
>>> V2 changes:
>>>
>>> Correct CC list information.
>>>
>>>
>>> V1 history:
>>>
>>> The series will resolve a buffer cross boundary access issue during the
>>> use of RAM disks. It is the mitigation for issue CVE-2018-12180.
>>>
>>> Cc: Jian J Wang <jian.j.wang@intel.com>
>>> Cc: Ray Ni <ray.ni@intel.com>
>>> Cc: Star Zeng <star.zeng@intel.com>
>>>
>>> Hao Wu (2):
>>>   MdeModulePkg/PartitionDxe: Ensure blocksize can hold MBR (CVE FIX)
>>>   MdeModulePkg/RamDiskDxe: Ramdisk size be multiple of BlkSize (CVE
>> FIX)
>>>
>>>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h     |  6 +++---
>>>  MdeModulePkg/Universal/Disk/PartitionDxe/Gpt.c           |  9 ++++++++-
>>>  MdeModulePkg/Universal/Disk/PartitionDxe/Mbr.c           |  9 ++++++++-
>>>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskBlockIo.c  | 20
>> ++++++++++++++------
>>>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c |  5 +++--
>>>  5 files changed, 36 insertions(+), 13 deletions(-)
>>>
>>
>> Please put the exact CVE numbers in the subject lines.
> 
> Hello Laszlo and Liming,
> 
> I totally agree the commit subject line should include the CVE number.
> But I have one feedback that, if the commit is for a CVE fix, is it
> possible to exempt the commit subject from 71 characters limit?

In my opinion, that is absolutely the case.

> I found it can be hard to summary the commit with the Package/Module plus
> the CVE number information.

I agree, it is hard. But, IMO, in this case, the precise CVE reference
takes priority.

Thanks
Laszlo

Re: [PATCH v2 0/2] MdeModulePkg: Resolve buffer cross boundary access in Ramdisk

[Gao, Liming]

Laszlo:
  I add my comments. 

Thanks
Liming
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Wednesday, February 27, 2019 4:58 PM
> To: Wu, Hao A <hao.a.wu@intel.com>; Gao, Liming <liming.gao@intel.com>; edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>
> Subject: Re: [edk2] [PATCH v2 0/2] MdeModulePkg: Resolve buffer cross boundary access in Ramdisk
> 
> On 02/27/19 07:56, Wu, Hao A wrote:
> >> -----Original Message-----
> >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> >> Laszlo Ersek
> >> Sent: Tuesday, February 26, 2019 7:45 PM
> >> To: Wu, Hao A; edk2-devel@lists.01.org
> >> Cc: Zeng, Star
> >> Subject: Re: [edk2] [PATCH v2 0/2] MdeModulePkg: Resolve buffer cross
> >> boundary access in Ramdisk
> >>
> >> On 02/26/19 08:45, Hao Wu wrote:
> >>> V2 changes:
> >>>
> >>> Correct CC list information.
> >>>
> >>>
> >>> V1 history:
> >>>
> >>> The series will resolve a buffer cross boundary access issue during the
> >>> use of RAM disks. It is the mitigation for issue CVE-2018-12180.
> >>>
> >>> Cc: Jian J Wang <jian.j.wang@intel.com>
> >>> Cc: Ray Ni <ray.ni@intel.com>
> >>> Cc: Star Zeng <star.zeng@intel.com>
> >>>
> >>> Hao Wu (2):
> >>>   MdeModulePkg/PartitionDxe: Ensure blocksize can hold MBR (CVE FIX)
> >>>   MdeModulePkg/RamDiskDxe: Ramdisk size be multiple of BlkSize (CVE
> >> FIX)
> >>>
> >>>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h     |  6 +++---
> >>>  MdeModulePkg/Universal/Disk/PartitionDxe/Gpt.c           |  9 ++++++++-
> >>>  MdeModulePkg/Universal/Disk/PartitionDxe/Mbr.c           |  9 ++++++++-
> >>>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskBlockIo.c  | 20
> >> ++++++++++++++------
> >>>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c |  5 +++--
> >>>  5 files changed, 36 insertions(+), 13 deletions(-)
> >>>
> >>
> >> Please put the exact CVE numbers in the subject lines.
> >
> > Hello Laszlo and Liming,
> >
> > I totally agree the commit subject line should include the CVE number.
> > But I have one feedback that, if the commit is for a CVE fix, is it
> > possible to exempt the commit subject from 71 characters limit?
> 
> In my opinion, that is absolutely the case.
> 
> > I found it can be hard to summary the commit with the Package/Module plus
> > the CVE number information.
> 
> I agree, it is hard. But, IMO, in this case, the precise CVE reference
> takes priority.
> 
For this case, I suggest to allow subject line length to be bigger, such as 120 character.
I will update wiki https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format for CVE commit message format. 
For example: Pkg-Module: Brief-single-line-summary (CVE-Year-Number)

> Thanks
> Laszlo

Re: [PATCH v2 0/2] MdeModulePkg: Resolve buffer cross boundary access in Ramdisk

[Laszlo Ersek]

On 02/27/19 13:49, Gao, Liming wrote:
> Laszlo:
>   I add my comments. 
> 
> Thanks
> Liming
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Wednesday, February 27, 2019 4:58 PM
>> To: Wu, Hao A <hao.a.wu@intel.com>; Gao, Liming <liming.gao@intel.com>; edk2-devel@lists.01.org
>> Cc: Zeng, Star <star.zeng@intel.com>
>> Subject: Re: [edk2] [PATCH v2 0/2] MdeModulePkg: Resolve buffer cross boundary access in Ramdisk
>>
>> On 02/27/19 07:56, Wu, Hao A wrote:
>>>> -----Original Message-----
>>>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
>>>> Laszlo Ersek
>>>> Sent: Tuesday, February 26, 2019 7:45 PM
>>>> To: Wu, Hao A; edk2-devel@lists.01.org
>>>> Cc: Zeng, Star
>>>> Subject: Re: [edk2] [PATCH v2 0/2] MdeModulePkg: Resolve buffer cross
>>>> boundary access in Ramdisk
>>>>
>>>> On 02/26/19 08:45, Hao Wu wrote:
>>>>> V2 changes:
>>>>>
>>>>> Correct CC list information.
>>>>>
>>>>>
>>>>> V1 history:
>>>>>
>>>>> The series will resolve a buffer cross boundary access issue during the
>>>>> use of RAM disks. It is the mitigation for issue CVE-2018-12180.
>>>>>
>>>>> Cc: Jian J Wang <jian.j.wang@intel.com>
>>>>> Cc: Ray Ni <ray.ni@intel.com>
>>>>> Cc: Star Zeng <star.zeng@intel.com>
>>>>>
>>>>> Hao Wu (2):
>>>>>   MdeModulePkg/PartitionDxe: Ensure blocksize can hold MBR (CVE FIX)
>>>>>   MdeModulePkg/RamDiskDxe: Ramdisk size be multiple of BlkSize (CVE
>>>> FIX)
>>>>>
>>>>>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h     |  6 +++---
>>>>>  MdeModulePkg/Universal/Disk/PartitionDxe/Gpt.c           |  9 ++++++++-
>>>>>  MdeModulePkg/Universal/Disk/PartitionDxe/Mbr.c           |  9 ++++++++-
>>>>>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskBlockIo.c  | 20
>>>> ++++++++++++++------
>>>>>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c |  5 +++--
>>>>>  5 files changed, 36 insertions(+), 13 deletions(-)
>>>>>
>>>>
>>>> Please put the exact CVE numbers in the subject lines.
>>>
>>> Hello Laszlo and Liming,
>>>
>>> I totally agree the commit subject line should include the CVE number.
>>> But I have one feedback that, if the commit is for a CVE fix, is it
>>> possible to exempt the commit subject from 71 characters limit?
>>
>> In my opinion, that is absolutely the case.
>>
>>> I found it can be hard to summary the commit with the Package/Module plus
>>> the CVE number information.
>>
>> I agree, it is hard. But, IMO, in this case, the precise CVE reference
>> takes priority.
>>
> For this case, I suggest to allow subject line length to be bigger, such as 120 character.
> I will update wiki https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format for CVE commit message format. 
> For example: Pkg-Module: Brief-single-line-summary (CVE-Year-Number)

Thanks for that!
Laszlo

Re: [PATCH v2 0/2] MdeModulePkg: Resolve buffer cross boundary access in Ramdisk

[Gao, Liming]

I update https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format with CVE example. Please check it. 

>-----Original Message-----
>From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
>Laszlo Ersek
>Sent: Thursday, February 28, 2019 3:31 AM
>To: Gao, Liming <liming.gao@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
>edk2-devel@lists.01.org
>Cc: Zeng, Star <star.zeng@intel.com>
>Subject: Re: [edk2] [PATCH v2 0/2] MdeModulePkg: Resolve buffer cross
>boundary access in Ramdisk
>
>On 02/27/19 13:49, Gao, Liming wrote:
>> Laszlo:
>>   I add my comments.
>>
>> Thanks
>> Liming
>>> -----Original Message-----
>>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>>> Sent: Wednesday, February 27, 2019 4:58 PM
>>> To: Wu, Hao A <hao.a.wu@intel.com>; Gao, Liming
><liming.gao@intel.com>; edk2-devel@lists.01.org
>>> Cc: Zeng, Star <star.zeng@intel.com>
>>> Subject: Re: [edk2] [PATCH v2 0/2] MdeModulePkg: Resolve buffer cross
>boundary access in Ramdisk
>>>
>>> On 02/27/19 07:56, Wu, Hao A wrote:
>>>>> -----Original Message-----
>>>>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf
>Of
>>>>> Laszlo Ersek
>>>>> Sent: Tuesday, February 26, 2019 7:45 PM
>>>>> To: Wu, Hao A; edk2-devel@lists.01.org
>>>>> Cc: Zeng, Star
>>>>> Subject: Re: [edk2] [PATCH v2 0/2] MdeModulePkg: Resolve buffer
>cross
>>>>> boundary access in Ramdisk
>>>>>
>>>>> On 02/26/19 08:45, Hao Wu wrote:
>>>>>> V2 changes:
>>>>>>
>>>>>> Correct CC list information.
>>>>>>
>>>>>>
>>>>>> V1 history:
>>>>>>
>>>>>> The series will resolve a buffer cross boundary access issue during the
>>>>>> use of RAM disks. It is the mitigation for issue CVE-2018-12180.
>>>>>>
>>>>>> Cc: Jian J Wang <jian.j.wang@intel.com>
>>>>>> Cc: Ray Ni <ray.ni@intel.com>
>>>>>> Cc: Star Zeng <star.zeng@intel.com>
>>>>>>
>>>>>> Hao Wu (2):
>>>>>>   MdeModulePkg/PartitionDxe: Ensure blocksize can hold MBR (CVE
>FIX)
>>>>>>   MdeModulePkg/RamDiskDxe: Ramdisk size be multiple of BlkSize
>(CVE
>>>>> FIX)
>>>>>>
>>>>>>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h     |  6
>+++---
>>>>>>  MdeModulePkg/Universal/Disk/PartitionDxe/Gpt.c           |  9
>++++++++-
>>>>>>  MdeModulePkg/Universal/Disk/PartitionDxe/Mbr.c           |  9
>++++++++-
>>>>>>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskBlockIo.c  | 20
>>>>> ++++++++++++++------
>>>>>>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c |  5
>+++--
>>>>>>  5 files changed, 36 insertions(+), 13 deletions(-)
>>>>>>
>>>>>
>>>>> Please put the exact CVE numbers in the subject lines.
>>>>
>>>> Hello Laszlo and Liming,
>>>>
>>>> I totally agree the commit subject line should include the CVE number.
>>>> But I have one feedback that, if the commit is for a CVE fix, is it
>>>> possible to exempt the commit subject from 71 characters limit?
>>>
>>> In my opinion, that is absolutely the case.
>>>
>>>> I found it can be hard to summary the commit with the Package/Module
>plus
>>>> the CVE number information.
>>>
>>> I agree, it is hard. But, IMO, in this case, the precise CVE reference
>>> takes priority.
>>>
>> For this case, I suggest to allow subject line length to be bigger, such as 120
>character.
>> I will update wiki
>https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-
>Format for CVE commit message format.
>> For example: Pkg-Module: Brief-single-line-summary (CVE-Year-Number)
>
>Thanks for that!
>Laszlo
>_______________________________________________
>edk2-devel mailing list
>edk2-devel@lists.01.org
>https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH v2 0/2] MdeModulePkg: Resolve buffer cross boundary access in Ramdisk

[Laszlo Ersek]

On 02/28/19 02:32, Gao, Liming wrote:
> I update https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format with CVE example. Please check it. 

"CVE fix needs to append CVE number in Brief-single-line-summary. The
format is 'Pkg-Module: Brief-single-line-summary (CVE-Year-Number)'. Its
length should be less than 92 characters."

Let's use the following suffix as example:

" (CVE-2018-12180)"

(the Number part is supposed to fit into 5 digits)

The length of this suffix is 17 characters. For normal cases, we have an
inclusive limit of 74 characters. So for CVE subjects the inclusive
limit is 74+17=91 characters. The wiki page states an exclusive limit of
92 chars, which is the same.

So, I think the update is perfect.

Thanks
Laszlo