From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by mx.groups.io with SMTP id smtpd.web10.1471.1572421984208445206 for ; Wed, 30 Oct 2019 00:53:04 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.20, mailfrom: shenglei.zhang@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 30 Oct 2019 00:53:03 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.68,246,1569308400"; d="scan'208";a="199154559" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by fmsmga007.fm.intel.com with ESMTP; 30 Oct 2019 00:53:02 -0700 Received: from fmsmsx161.amr.corp.intel.com (10.18.125.9) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 30 Oct 2019 00:53:02 -0700 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by FMSMSX161.amr.corp.intel.com (10.18.125.9) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 30 Oct 2019 00:53:02 -0700 Received: from shsmsx106.ccr.corp.intel.com ([169.254.10.248]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.60]) with mapi id 14.03.0439.000; Wed, 30 Oct 2019 15:53:00 +0800 From: "Zhang, Shenglei" To: "Wu, Hao A" , "devel@edk2.groups.io" CC: "Ni, Ray" Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/SdBlockIoPei: Add check for DeviceIndex Thread-Topic: [edk2-devel] [PATCH] MdeModulePkg/SdBlockIoPei: Add check for DeviceIndex Thread-Index: AQHVhLMbjK2mAdUEM0K+pujkBWqoeKdy2JwQgAALaRA= Date: Wed, 30 Oct 2019 07:53:00 +0000 Message-ID: References: <20191017062111.10568-1-shenglei.zhang@intel.com> <20191017062111.10568-2-shenglei.zhang@intel.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: shenglei.zhang@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable > -----Original Message----- > From: Wu, Hao A > Sent: Wednesday, October 30, 2019 3:15 PM > To: devel@edk2.groups.io; Zhang, Shenglei > Cc: Ni, Ray > Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/SdBlockIoPei: Add check > for DeviceIndex >=20 > > -----Original Message----- > > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > > Zhang, Shenglei > > Sent: Thursday, October 17, 2019 2:21 PM > > To: devel@edk2.groups.io > > Cc: Wu, Hao A; Ni, Ray > > Subject: [edk2-devel] [PATCH] MdeModulePkg/SdBlockIoPei: Add check > for > > DeviceIndex > > > > DeviceIndex is used as index in Slot[]. The max size of Slot[] > > is SD_PEIM_MAX_SLOTS. So DeviceIndex should be checked before used. > > > > Cc: Hao A Wu > > Cc: Ray Ni > > Signed-off-by: Shenglei Zhang > > --- > > MdeModulePkg/Bus/Sd/SdBlockIoPei/SdBlockIoPei.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/MdeModulePkg/Bus/Sd/SdBlockIoPei/SdBlockIoPei.c > > b/MdeModulePkg/Bus/Sd/SdBlockIoPei/SdBlockIoPei.c > > index 8fa58d65b22c..25530dcb34ce 100644 > > --- a/MdeModulePkg/Bus/Sd/SdBlockIoPei/SdBlockIoPei.c > > +++ b/MdeModulePkg/Bus/Sd/SdBlockIoPei/SdBlockIoPei.c > > @@ -174,7 +174,7 @@ SdBlockIoPeimGetMediaInfo ( > > > > Private =3D GET_SD_PEIM_HC_PRIVATE_DATA_FROM_THIS (This); > > > > - if ((DeviceIndex =3D=3D 0) || (DeviceIndex > Private->TotalBlkIoDev= ices)) { > > + if ((DeviceIndex =3D=3D 0) || (DeviceIndex > Private->TotalBlkIoDev= ices) || > > (DeviceIndex > (SD_PEIM_MAX_SLOTS - 1))) { >=20 >=20 > Hello, >=20 > I do not think the change is proper, since 'DeviceIndex' is used to acce= ss the > array Private->Slot[SD_PEIM_MAX_SLOTS] like: >=20 > Private->Slot[DeviceIndex - 1] >=20 > I think the change should be: >=20 > ... || (DeviceIndex > (SD_PEIM_MAX_SLOTS) >=20 > instead of: >=20 > ... || (DeviceIndex > (SD_PEIM_MAX_SLOTS - 1) >=20 >=20 > Could you help to double confirm on this? Thanks in advance. >=20 Hao, You are right. The index used below the if statement is " DeviceIndex -1" = not " DeviceIndex ". Thanks, Shenglei > Best Regards, > Hao Wu >=20 >=20 > > return EFI_INVALID_PARAMETER; > > } > > > > @@ -252,7 +252,7 @@ SdBlockIoPeimReadBlocks ( > > return EFI_SUCCESS; > > } > > > > - if ((DeviceIndex =3D=3D 0) || (DeviceIndex > Private->TotalBlkIoDev= ices)) { > > + if ((DeviceIndex =3D=3D 0) || (DeviceIndex > Private->TotalBlkIoDev= ices) || > > (DeviceIndex > (SD_PEIM_MAX_SLOTS - 1))) { > > return EFI_INVALID_PARAMETER; > > } > > > > -- > > 2.18.0.windows.1 > > > > > >=20