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

[Laszlo Ersek]

On 03/27/21 00:41, Kun Qin wrote:
> 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 at intel.com>
> Cc: Ray Ni <ray.ni at intel.com>
> Cc: Laszlo Ersek <lersek at redhat.com>
> Cc: Rahul Kumar <rahul1.kumar at intel.com>
> 
> Signed-off-by: Kun Qin <kuqin12 at 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
>      //
> 

Please update the description of the EFI_INVALID_PARAMETER return code
in the function's leading comment as well.

With that:

Reviewed-by: Laszlo Ersek <lersek at redhat.com>

Thanks
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73721): https://edk2.groups.io/g/devel/message/73721
Mute This Topic: https://groups.io/mt/81642500/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-