Re: [Patch] NetworkPkg/IpSecDxe: Fix issue to parse SA Payload.

["Wu, Jiaxin" <jiaxin.wu@intel.com>]

Thanks Ting, I will update the comments against the function.



> -----Original Message-----
> From: Ye, Ting
> Sent: Thursday, October 18, 2018 11:26 AM
> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> Cc: Fu, Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> Subject: RE: [edk2] [Patch] NetworkPkg/IpSecDxe: Fix issue to parse SA
> Payload.
> 
> Hi Jiaxin,
> 
> I am confused why we need set values to following local variables when
> Ikev2ParseProposalData marks them as 'out' attribute. Please adds more
> comments why '0' is required and updates 'out' to 'in out' if '0' is necessary.
> 
> +        IntegrityAlgorithm = 0;
> +        EncryptAlgorithm   = 0;
> +        EncryptKeylength   = 0;
> +        IsSupportEsn       = FALSE;
> 
> Thanks,
> Ting
> 
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Jiaxin Wu
> Sent: Tuesday, October 16, 2018 9:34 AM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Wu,
> Jiaxin <jiaxin.wu@intel.com>
> Subject: [edk2] [Patch] NetworkPkg/IpSecDxe: Fix issue to parse SA Payload.
> 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1251
> 
> IpSecDxe failed to create the Child SA during parsing SA Payload, the issue
> was caused by the below commit:
> 
> SHA-1: 1e0db7b11987d0ec93be7dfe26102a327860fdbd
> * MdeModulePkg/NetworkPkg: Checking for NULL pointer before use.
> 
> In above commit, it changed the value of IsMatch in
> Ikev2ChildSaParseSaPayload() to FALSE. That's correct but it exposed the
> potential bug in to match the correct proposal Data, which will cause the
> issue happen.
> 
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> ---
>  NetworkPkg/IpSecDxe/Ikev2/Utility.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/NetworkPkg/IpSecDxe/Ikev2/Utility.c
> b/NetworkPkg/IpSecDxe/Ikev2/Utility.c
> index 0c9c929705..d61bae8c9d 100644
> --- a/NetworkPkg/IpSecDxe/Ikev2/Utility.c
> +++ b/NetworkPkg/IpSecDxe/Ikev2/Utility.c
> @@ -2502,15 +2502,16 @@ Ikev2ChildSaParseSaPayload (
>            IntegrityAlgorithm == PreferIntegrityAlgorithm &&
>            IsSupportEsn == PreferIsSupportEsn
>            ) {
>          IsMatch = TRUE;
>        } else {
> -        PreferEncryptAlgorithm   = 0;
> -        PreferIntegrityAlgorithm = 0;
> -        IsSupportEsn             = TRUE;
> +        IntegrityAlgorithm = 0;
> +        EncryptAlgorithm   = 0;
> +        EncryptKeylength   = 0;
> +        IsSupportEsn       = FALSE;
>        }
> -       ProposalData = (IKEV2_PROPOSAL_DATA*)((UINT8*)(ProposalData + 1)
> +
> +      ProposalData = (IKEV2_PROPOSAL_DATA*)((UINT8*)(ProposalData + 1)
> + +
>                       ProposalData->NumTransforms * sizeof
> (IKEV2_TRANSFORM_DATA));
>      }
> 
>      ProposalData  = (IKEV2_PROPOSAL_DATA *)((IKEV2_SA_DATA
> *)SaPayload->PayloadBuf + 1);
>      if (IsMatch) {
> --
> 2.17.1.windows.2
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel