MdeModulePkg/Bus/Sd/SdBlockIoPei/SdBlockIoPei.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
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 <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
---
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 = GET_SD_PEIM_HC_PRIVATE_DATA_FROM_THIS (This);
- if ((DeviceIndex == 0) || (DeviceIndex > Private->TotalBlkIoDevices)) {
+ if ((DeviceIndex == 0) || (DeviceIndex > Private->TotalBlkIoDevices) || (DeviceIndex > (SD_PEIM_MAX_SLOTS - 1))) {
return EFI_INVALID_PARAMETER;
}
@@ -252,7 +252,7 @@ SdBlockIoPeimReadBlocks (
return EFI_SUCCESS;
}
- if ((DeviceIndex == 0) || (DeviceIndex > Private->TotalBlkIoDevices)) {
+ if ((DeviceIndex == 0) || (DeviceIndex > Private->TotalBlkIoDevices) || (DeviceIndex > (SD_PEIM_MAX_SLOTS - 1))) {
return EFI_INVALID_PARAMETER;
}
--
2.18.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#49154): https://edk2.groups.io/g/devel/message/49154
Mute This Topic: https://groups.io/mt/34685173/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
> -----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 <hao.a.wu@intel.com> > Cc: Ray Ni <ray.ni@intel.com> > Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com> > --- > 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 = GET_SD_PEIM_HC_PRIVATE_DATA_FROM_THIS (This); > > - if ((DeviceIndex == 0) || (DeviceIndex > Private->TotalBlkIoDevices)) { > + if ((DeviceIndex == 0) || (DeviceIndex > Private->TotalBlkIoDevices) || > (DeviceIndex > (SD_PEIM_MAX_SLOTS - 1))) { Hello, I do not think the change is proper, since 'DeviceIndex' is used to access the array Private->Slot[SD_PEIM_MAX_SLOTS] like: Private->Slot[DeviceIndex - 1] I think the change should be: ... || (DeviceIndex > (SD_PEIM_MAX_SLOTS) instead of: ... || (DeviceIndex > (SD_PEIM_MAX_SLOTS - 1) Could you help to double confirm on this? Thanks in advance. Best Regards, Hao Wu > return EFI_INVALID_PARAMETER; > } > > @@ -252,7 +252,7 @@ SdBlockIoPeimReadBlocks ( > return EFI_SUCCESS; > } > > - if ((DeviceIndex == 0) || (DeviceIndex > Private->TotalBlkIoDevices)) { > + if ((DeviceIndex == 0) || (DeviceIndex > Private->TotalBlkIoDevices) || > (DeviceIndex > (SD_PEIM_MAX_SLOTS - 1))) { > return EFI_INVALID_PARAMETER; > } > > -- > 2.18.0.windows.1 > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#49641): https://edk2.groups.io/g/devel/message/49641 Mute This Topic: https://groups.io/mt/34685173/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
> -----Original Message----- > From: Wu, Hao A > Sent: Wednesday, October 30, 2019 3:15 PM > To: devel@edk2.groups.io; Zhang, Shenglei <shenglei.zhang@intel.com> > Cc: Ni, Ray <ray.ni@intel.com> > Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/SdBlockIoPei: Add check > for DeviceIndex > > > -----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 <hao.a.wu@intel.com> > > Cc: Ray Ni <ray.ni@intel.com> > > Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com> > > --- > > 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 = GET_SD_PEIM_HC_PRIVATE_DATA_FROM_THIS (This); > > > > - if ((DeviceIndex == 0) || (DeviceIndex > Private->TotalBlkIoDevices)) { > > + if ((DeviceIndex == 0) || (DeviceIndex > Private->TotalBlkIoDevices) || > > (DeviceIndex > (SD_PEIM_MAX_SLOTS - 1))) { > > > Hello, > > I do not think the change is proper, since 'DeviceIndex' is used to access the > array Private->Slot[SD_PEIM_MAX_SLOTS] like: > > Private->Slot[DeviceIndex - 1] > > I think the change should be: > > ... || (DeviceIndex > (SD_PEIM_MAX_SLOTS) > > instead of: > > ... || (DeviceIndex > (SD_PEIM_MAX_SLOTS - 1) > > > Could you help to double confirm on this? Thanks in advance. > Hao, You are right. The index used below the if statement is " DeviceIndex -1" not " DeviceIndex ". Thanks, Shenglei > Best Regards, > Hao Wu > > > > return EFI_INVALID_PARAMETER; > > } > > > > @@ -252,7 +252,7 @@ SdBlockIoPeimReadBlocks ( > > return EFI_SUCCESS; > > } > > > > - if ((DeviceIndex == 0) || (DeviceIndex > Private->TotalBlkIoDevices)) { > > + if ((DeviceIndex == 0) || (DeviceIndex > Private->TotalBlkIoDevices) || > > (DeviceIndex > (SD_PEIM_MAX_SLOTS - 1))) { > > return EFI_INVALID_PARAMETER; > > } > > > > -- > > 2.18.0.windows.1 > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#49645): https://edk2.groups.io/g/devel/message/49645 Mute This Topic: https://groups.io/mt/34685173/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.