Okay.

Best Regards,
Hao Wu

From: Zhang, Chao B
Sent: Friday, June 28, 2019 9:07 AM
To: Wu, Hao A; Xu, Wei6; devel@edk2.groups.io
Cc: Wang, Jian J
Subject: RE: [edk2-devel][Patch] MdeModulePkg/DxeCapsuleLibFmp: Add missing 
NULL pointer check.

HI Hao:
   I think the patch is to complete the security check both in info and code 
logic to ValidateCapsuleNameCapsuleIntegrity .
It is OK to keep it in one patch.


From: Wu, Hao A
Sent: Friday, June 28, 2019 8:54 AM
To: Xu, Wei6 <wei6...@intel.com>; devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.w...@intel.com>; Zhang, Chao B <chao.b.zh...@intel.com>
Subject: RE: [edk2-devel][Patch] MdeModulePkg/DxeCapsuleLibFmp: Add missing 
NULL pointer check.

> -----Original Message-----
> From: Xu, Wei6
> Sent: Friday, June 28, 2019 12:26 AM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> Cc: Wang, Jian J; Wu, Hao A; Zhang, Chao B
> Subject: [edk2-devel][Patch] MdeModulePkg/DxeCapsuleLibFmp: Add
> missing NULL pointer check.
>
> Add missing NULL pointer check for CapsuleNameBufStart.
> Also add comments to notice that capsule name integrity check assumes
> the capsule has been validated by IsValidCapsuleHeader().


The patch is doing 2 things. Please help to split it into 2 commits.

With this handled,
Reviewed-by: Hao A Wu <hao.a...@intel.com<mailto:hao.a...@intel.com>>

Best Regards,
Hao Wu


>
> Cc: Jian J Wang <jian.j.w...@intel.com<mailto:jian.j.w...@intel.com>>
> Cc: Hao A Wu <hao.a...@intel.com<mailto:hao.a...@intel.com>>
> Cc: Chao B Zhang <chao.b.zh...@intel.com<mailto:chao.b.zh...@intel.com>>
> Signed-off-by: Wei6 Xu <wei6...@intel.com<mailto:wei6...@intel.com>>
> ---
>  MdeModulePkg/Library/DxeCapsuleLibFmp/CapsuleOnDisk.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/CapsuleOnDisk.c
> b/MdeModulePkg/Library/DxeCapsuleLibFmp/CapsuleOnDisk.c
> index 66c9be8e1f..3193ca8f4d 100644
> --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/CapsuleOnDisk.c
> +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/CapsuleOnDisk.c
> @@ -23,10 +23,13 @@ IsCapsuleNameCapsule (
>
>  /**
>    Check the integrity of the capsule name capsule.
>    If the capsule is vaild, return the physical address of each capsule name
> string.
>
> +  This routine assumes the capsule has been validated by
> IsValidCapsuleHeader(), so
> +  capsule memory overflow is not going to happen in this routine.
> +
>    @param[in]  CapsuleHeader   Pointer to the capsule header of a capsule
> name capsule.
>    @param[out] CapsuleNameNum  Number of capsule name.
>
>    @retval NULL                Capsule name capsule is not valid.
>    @retval CapsuleNameBuf      Array of capsule name physical address.
> @@ -63,10 +66,13 @@ ValidateCapsuleNameCapsuleIntegrity (
>    //
>    // If strings are not aligned on a 16-bit boundary, reallocate memory for 
> it.
>    //
>    if (((UINTN) CapsuleNameBufStart & BIT0) != 0) {
>      CapsuleNameBufStart = AllocateCopyPool (CapsuleHeader-
> >CapsuleImageSize - CapsuleHeader->HeaderSize, CapsuleNameBufStart);
> +    if (CapsuleNameBufStart == NULL) {
> +      return NULL;
> +    }
>    }
>
>    CapsuleNameBufEnd = CapsuleNameBufStart + CapsuleHeader-
> >CapsuleImageSize - CapsuleHeader->HeaderSize;
>
>    CapsuleNamePtr = CapsuleNameBufStart;
> --
> 2.16.2.windows.1

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#42953): https://edk2.groups.io/g/devel/message/42953
Mute This Topic: https://groups.io/mt/32232931/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to