Re: [edk2-devel] [PATCH v1 1/1] UefiCpuPkg: PiSmmCpuDxeSmm: Check buffer size before accessing

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

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

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Kun Qin
> Sent: Saturday, March 27, 2021 7:42 AM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>; Kumar, Rahul1
> <rahul1.kumar@intel.com>
> Subject: [edk2-devel] [PATCH v1 1/1] UefiCpuPkg: PiSmmCpuDxeSmm: Check buffer size before accessing
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3283
> 
> Current SMM Save State routine does not check the number of bytes to be
> read, when it comse to read IO_INFO, before casting the incoming buffer
> to EFI_SMM_SAVE_STATE_IO_INFO. This could potentially cause memory
> corruption due to extra bytes are written out of buffer boundary.
> 
> This change adds a width check before copying IoInfo into output buffer.
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> 
> Signed-off-by: Kun Qin <kuqin12@gmail.com>
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c
> index 661cc51f361a..ec760e4c37ca 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c
> @@ -418,6 +418,13 @@ ReadSaveStateRegister (
>        return EFI_NOT_FOUND;
>      }
> 
> +    //
> +    // Make sure the incoming buffer is large enough to hold IoInfo before accessing
> +    //
> +    if (Width < sizeof (EFI_SMM_SAVE_STATE_IO_INFO)) {
> +      return EFI_INVALID_PARAMETER;
> +    }
> +
>      //
>      // Zero the IoInfo structure that will be returned in Buffer
>      //
> --
> 2.31.0.windows.1
> 
> 
> 
> 
>