From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM11-CO1-obe.outbound.protection.outlook.com (NAM11-CO1-obe.outbound.protection.outlook.com [40.107.220.91]) by mx.groups.io with SMTP id smtpd.web10.48013.1584949203879520276 for ; Mon, 23 Mar 2020 00:40:04 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@microsoft.com header.s=selector2 header.b=e+AKPEgk; spf=pass (domain: microsoft.com, ip: 40.107.220.91, mailfrom: kun.qin@microsoft.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=EcxIzQs5JS0hsY4zjTxaB9qLBjpWqTtyFOyWY+Ywnyi5to8B1YADt6g+20QrN4IeL9jtWSwu9DQT8naSXn7lLln+KLHqXTrXGZ0SHRiqhiUkrMKsYCu3w8+4SWVIM7O4E1BAtIa6t/vOPvxaa8BTso7LbCLV5niv0KENXjWF2y5Z2e3Y6T2jxvKf30dHPa/9u7U/XWU1LAg58XMPmaaqd6ydd4Lsu07J00NMlREZhJrYr2a7mpaYSEo7QyNlCulCpomMwX+iUzY+6POn8ueAG3elDi2JpCBxhF+ZLvZps0d9Ix9TGXl+T7bVDFFhdbkH4zxqyatzx9BoC27PDAHDGQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=SIJp03FKc4mHzbLs2wH7MrgE3usPvFHhJ+jylfJjJ5w=; b=UeyGVuKbYlL7FLQhi+r1uRcHCLFjFgBg4bKuOVeLmnrN5naCcFprAulrM/RBaGWuuIb4CrySNQX6hHgFtk2f+0T7gI5cyB3yV7NJ87tNvC9Oy5qD5oRsRcGhH2ixudNK1HTkDEjenqO7yrETOnIruNDWTDwdWOIQrf/1I8XYrRfCsMO6e2JOZ3ZDt5D5exoB6GK786wySCLManCHyFuGp/1ay3Kh9DV4OWPIswhv8tQeVjICxvmMRkSPFt+7R0DoZBXQyvdrmH9aWuRRbgj2nXM/GWG9b9YviHpVqN7zrGSJz08pQOt0rQPERpGF5cPjzXlDqgxq3CmIHy0pJndYxg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=microsoft.com; dmarc=pass action=none header.from=microsoft.com; dkim=pass header.d=microsoft.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=SIJp03FKc4mHzbLs2wH7MrgE3usPvFHhJ+jylfJjJ5w=; b=e+AKPEgkwgCM1ges0wdlKj/4OnLyw6FrqvEvH/HRwVe0b6u4KOAyVLhoSp1VKKs892P2D8CF+bPBPTjqZbJtIn5Qyj5lbsKUYwSPVMjvS67bxWYsnbHt1OGMifEW/a1wv6b7OEfP1LmvjnJoZ7CJuAe3YC/gEAxMqjU9LE20+N4= Received: from MWHPR21MB0192.namprd21.prod.outlook.com (2603:10b6:300:79::10) by MWHPR21MB0144.namprd21.prod.outlook.com (2603:10b6:300:78::14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2878.2; Mon, 23 Mar 2020 07:40:02 +0000 Received: from MWHPR21MB0192.namprd21.prod.outlook.com ([fe80::f010:3582:2203:7510]) by MWHPR21MB0192.namprd21.prod.outlook.com ([fe80::f010:3582:2203:7510%8]) with mapi id 15.20.2878.000; Mon, 23 Mar 2020 07:40:02 +0000 From: Kun Qin To: "Jiang, Guomin" , "devel@edk2.groups.io" , "Kinney, Michael D" , "Xu, Wei6" CC: "Gao, Liming" Subject: Re: [edk2-devel] [PATCH] FmpDevicePkg/FmpDxe: Fix uninitialized pointer dereference Thread-Topic: [edk2-devel] [PATCH] FmpDevicePkg/FmpDxe: Fix uninitialized pointer dereference Thread-Index: AQHV/TgJQmvSGvcwJkKhpHdU8+tdsqhVzOSAgAACdd0= Date: Mon, 23 Mar 2020 07:40:02 +0000 Message-ID: References: <20200318061227.12480-1-wei6.xu@intel.com> , In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: msip_labels: MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Enabled=True;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SiteId=72f988bf-86f1-41af-91ab-2d7cd011db47;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SetDate=2020-03-23T07:30:14.2426173Z;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_ContentBits=0;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Method=Privileged authentication-results: spf=none (sender IP is ) smtp.mailfrom=Kun.Qin@microsoft.com; x-originating-ip: [73.239.241.211] x-ms-publictraffictype: Email x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: e8d5550c-ae38-483c-4fdc-08d7cefd65c7 x-ms-traffictypediagnostic: MWHPR21MB0144:|MWHPR21MB0144: x-ld-processed: 72f988bf-86f1-41af-91ab-2d7cd011db47,ExtAddr x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:3276; x-forefront-prvs: 0351D213B3 x-forefront-antispam-report: SFV:NSPM;SFS:(10019020)(4636009)(376002)(396003)(346002)(366004)(136003)(39860400002)(199004)(66476007)(26005)(81156014)(81166006)(55016002)(966005)(10290500003)(7696005)(9326002)(8936002)(478600001)(4326008)(71200400001)(8676002)(9686003)(53546011)(186003)(64756008)(66446008)(66946007)(52536014)(6506007)(33656002)(66556008)(76116006)(8990500004)(2906002)(110136005)(86362001)(316002)(5660300002);DIR:OUT;SFP:1102;SCL:1;SRVR:MWHPR21MB0144;H:MWHPR21MB0192.namprd21.prod.outlook.com;FPR:;SPF:None;LANG:en;PTR:InfoNoRecords;A:1; received-spf: None (protection.outlook.com: microsoft.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: VOg+FdREhHgqdRdP3AMJm7VSmhtyoo1lxMSmlFBZZtnMCCF2JxRgsaUutxRIy59/NfY4967vuIUgM+B5bl7IjxAh/LFpj1LCK+rLFRmets7zr+24Wyy/me818z35jSu4MsDJ/1ghJQCp6rm+Omr44o3F6eMu1b/oFkigXvlJoK16yRn9gAB4l5Lnbwy8lU3X1foap/Sdax6v9RXkOlWJvqzptYTt7fQnNGlWChP1bYAwOEC6UBQO/UAUiauuITnQwHSaU8WcsnD8EjS4tpN2dupfCjVcMkbYx+Ua2m3vHiWCfyZSc/GSNgc2ATVDqnmM1zy/leBqTuNfuxEW7EO+hjrx7Sz38z9qEyo+ltTgNMqmkj9uS6FBwuo/HVYbKTtd902drbNkuwEiPzNIN6Ukdc++3RcTxWKmpac0KoWGDv6cWIt3DOLgdfAHJmAC16sL9T0Zh4dQwyOF62dv0C8OQcLXtntjTXZxoHoShNZxAt6uRdi2njWc+kQDPjxCGNthLnjV0nzrHOna5QB1H36bsQ== x-ms-exchange-antispam-messagedata: ihSeT781MXcFyKwkodAjgUcGFyAB8at7Vda4t3qpcixTCIl8xJFNUM/nPrAt3mNkwiKROVOTbobFpT3mxKyg85pKqHW/LIZB4NBpSiba/9wgyUdjd79z9MhBFvxmBGg18fWiY1rRsif70/4SmyRzUw== x-ms-exchange-transport-forked: True MIME-Version: 1.0 X-OriginatorOrg: microsoft.com X-MS-Exchange-CrossTenant-Network-Message-Id: e8d5550c-ae38-483c-4fdc-08d7cefd65c7 X-MS-Exchange-CrossTenant-originalarrivaltime: 23 Mar 2020 07:40:02.4353 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 72f988bf-86f1-41af-91ab-2d7cd011db47 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: GRbsqrfQd2Gj+S5x8FEyUxJD9qd8JZ/nCfIrSW48I0Kut437e6mYSo7D16JwxEtmxU+wO+hneguq84YiJ2A3Rg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR21MB0144 Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_MWHPR21MB0192C4CDD61430431BE746C5E9F00MWHPR21MB0192namp_" --_000_MWHPR21MB0192C4CDD61430431BE746C5E9F00MWHPR21MB0192namp_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Guomin, Thanks for reaching out. I did encounter a GP fault because of this issue: If Line 582 is triggered when = the first Fmp->GetImageInfo failed, this specific mFmpImageInfoBuf[Index] w= ill remain to be uninitialized value (0xFAFAFAFAFAF in my case). Later on w= hen it comes to line 632, it wi= ll pass the null pointer check and try to dereference it, which leads to GP= fault. Please let me know if you need further clarification. Thanks, Kun From: Jiang, Guomin Sent: Monday, March 23, 2020 12:21 AM To: devel@edk2.groups.io; Kinney, Michael D; Xu, Wei6 Cc: Kun Qin; Gao, Liming Subject: [EXTERNAL] RE: [edk2-devel] [PATCH] FmpDevicePkg/FmpDxe: Fix unin= itialized pointer dereference Hi Xuwei, QinKun, Have you indeed encounter this issue or just think it is potential issue. I think below code will always initialize the mFmpImageInfoBuf[] and make= sure it is valid. Line 585 - mFmpImageInfoBuf[Index] =3D AllocateZeroPool (ImageInfoSize); If the second GetImageInfo() is runned, I think it will always have correc= t mfmpImageInfoBuf[] address. Of course, it is ok to use AllocateZeroPool to ensure zero buffer is alloc= ated. Thanks > -----Original Message----- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Michael D Kinney > Sent: Wednesday, March 18, 2020 11:15 PM > To: Xu, Wei6 ; devel@edk2.groups.io; Kinney, Michael > D > Cc: Kun Qin ; Gao, Liming > Subject: Re: [edk2-devel] [PATCH] FmpDevicePkg/FmpDxe: Fix uninitialized > pointer dereference > > Reviewed-by: Michael D Kinney > > > -----Original Message----- > > From: Xu, Wei6 > > Sent: Tuesday, March 17, 2020 11:12 PM > > To: devel@edk2.groups.io > > Cc: Kun Qin ; Kinney, Michael D > > ; Gao, Liming > > Subject: [edk2-devel] [PATCH] FmpDevicePkg/FmpDxe: Fix uninitialized > > pointer dereference > > > > From: Kun Qin > > > > REF: > > https://nam06.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Fbu= gzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2602&data=3D02%7C01%7CKun.Qi= n%40microsoft.com%7C3c1042cd095b42a51b9d08d7cefad022%7C72f988bf86f141af91ab= 2d7cd011db47%7C1%7C0%7C637205448946602054&sdata=3D95z6fDC0uceCCs2MuoeCR= 4MXgRhAI3dVssWeddsWT5s%3D&reserved=3D0 > > > > Zero the allocated buffer in case GetImageInfo `continue` in the > > middle of a loop. This will cause unexpected GetImageInfo failure not > > clearing the corresponding entry and lead to GP faults when > > dereferencing this entry. > > > > Cc: Michael D Kinney > > Cc: Liming Gao > > Signed-off-by: Wei6 Xu > > --- > > FmpDevicePkg/FmpDxe/Dependency.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/FmpDevicePkg/FmpDxe/Dependency.c > > b/FmpDevicePkg/FmpDxe/Dependency.c > > index 8f97c42916..65c23989c6 100644 > > --- a/FmpDevicePkg/FmpDxe/Dependency.c > > +++ b/FmpDevicePkg/FmpDxe/Dependency.c > > @@ -550,11 +550,11 @@ EvaluateImageDependencies ( > > ); > > if (EFI_ERROR (Status)) { > > return EFI_ABORTED; > > } > > > > - mFmpImageInfoBuf =3D AllocatePool > > (sizeof(EFI_FIRMWARE_IMAGE_DESCRIPTOR *) * > mNumberOfFmpInstance); > > + mFmpImageInfoBuf =3D AllocateZeroPool > > (sizeof(EFI_FIRMWARE_IMAGE_DESCRIPTOR *) * > mNumberOfFmpInstance); > > if (mFmpImageInfoBuf =3D=3D NULL) { > > return EFI_OUT_OF_RESOURCES; > > } > > > > for (Index =3D 0; Index < mNumberOfFmpInstance; Index > > ++) { > > -- > > 2.16.2.windows.1 > > >=20 --_000_MWHPR21MB0192C4CDD61430431BE746C5E9F00MWHPR21MB0192namp_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable

Hi Guomin,

 

Thanks for reaching out. I did encounter a GP fault= because of this issue:

 

If Line 582 is triggered when the first Fmp->GetImageInfo failed, this= specific mFmpImageInfoBuf[Index] will remain to be uninitialized value (0xFA= FAFAFAFAF in my case). Later on when it comes to line 632, it will pass the null pointer check and try to dereference i= t, which leads to GP fault. Please let me know if you need further clarific= ation.

 

Thanks,

Kun

 

From: Jiang, Guomin
Sent: Monday, March 23, 2020 12:21 AM
To: devel@edk2.groups.io; Kinney, Michael D; Xu, Wei6 Cc: Kun Qin; Gao, Liming
Subject: [EXTERNAL] RE: [edk2-devel] [PATCH] FmpDevicePkg/FmpDxe: F= ix uninitialized pointer dereference

 

Hi Xuwei, QinKun,
Have you indeed encounter this issue or just think it is potential issue.<= br>
I think  below code will always initialize the mFmpImageInfoBuf[] and= make sure it is valid.
Line 585 - mFmpImageInfoBuf[Index] =3D AllocateZeroPool (ImageInfoSize);
If the second GetImageInfo() is runned, I think it will always have correc= t mfmpImageInfoBuf[] address.

Of course, it is ok to use AllocateZeroPool to ensure zero buffer is alloc= ated.

Thanks

> -----Original Message-----
> From: devel@edk2.groups.io [m= ailto:devel@edk2.groups.io] On Behalf Of
> Michael D Kinney
> Sent: Wednesday, March 18, 2020 11:15 PM
> To: Xu, Wei6 <wei6.xu@intel.com>; devel@edk2.groups.io; Kinney,= Michael
> D <michael.d.kinney@intel.com>
> Cc: Kun Qin <kuqin@microsoft.com>; Gao, Liming <liming.gao@i= ntel.com>
> Subject: Re: [edk2-devel] [PATCH] FmpDevicePkg/FmpDxe: Fix uninitiali= zed
> pointer dereference
>
> Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
>
> > -----Original Message-----
> > From: Xu, Wei6 <wei6.xu@intel.com>
> > Sent: Tuesday, March 17, 2020 11:12 PM
> > To: devel@edk2.groups.io
> > Cc: Kun Qin <kuqin@microsoft.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@i= ntel.com>
> > Subject: [edk2-devel] [PATCH] FmpDevicePkg/FmpDxe: Fix uninitial= ized
> > pointer dereference
> >
> > From: Kun Qin <kuqin@microsoft.com>
> >
> > REF:
> > https://nam06.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Fbugzil= la.tianocore.org%2Fshow_bug.cgi%3Fid%3D2602&amp;data=3D02%7C01%7CKun.Qi= n%40microsoft.com%7C3c1042cd095b42a51b9d08d7cefad022%7C72f988bf86f141af91ab= 2d7cd011db47%7C1%7C0%7C637205448946602054&amp;sdata=3D95z6fDC0uceCCs2Mu= oeCR4MXgRhAI3dVssWeddsWT5s%3D&amp;reserved=3D0
> >
> > Zero the allocated buffer in case GetImageInfo `continue` in the=
> > middle of a loop. This will cause unexpected GetImageInfo failur= e not
> > clearing the corresponding entry and lead to GP faults when
> > dereferencing this entry.
> >
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Signed-off-by: Wei6 Xu <wei6.xu@intel.com>
> > ---
> >  FmpDevicePkg/FmpDxe/Dependency.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/FmpDevicePkg/FmpDxe/Dependency.c
> > b/FmpDevicePkg/FmpDxe/Dependency.c
> > index 8f97c42916..65c23989c6 100644
> > --- a/FmpDevicePkg/FmpDxe/Dependency.c
> > +++ b/FmpDevicePkg/FmpDxe/Dependency.c
> > @@ -550,11 +550,11 @@ EvaluateImageDependencies (
> >           = ;       );
> >    if (EFI_ERROR (Status)) {
> >      return EFI_ABORTED;
> >    }
> >
> > -  mFmpImageInfoBuf =3D AllocatePool
> > (sizeof(EFI_FIRMWARE_IMAGE_DESCRIPTOR *) *
> mNumberOfFmpInstance);
> > +  mFmpImageInfoBuf =3D AllocateZeroPool
> > (sizeof(EFI_FIRMWARE_IMAGE_DESCRIPTOR *) *
> mNumberOfFmpInstance);
> >    if (mFmpImageInfoBuf =3D=3D NULL) {
> >      return EFI_OUT_OF_RESOURCES;
> >    }
> >
> >    for (Index =3D 0; Index < mNumberOfFmpInsta= nce; Index
> > ++) {
> > --
> > 2.16.2.windows.1
>
>
>

 

--_000_MWHPR21MB0192C4CDD61430431BE746C5E9F00MWHPR21MB0192namp_--