* [PATCH] MdeModulePkg/HiiDatabaseDxe: Add check for 'Private->Attribute >> 4' @ 2019-10-17 6:21 Zhang, Shenglei 2019-10-17 6:21 ` [PATCH] MdeModulePkg/SdBlockIoPei: Add check for DeviceIndex Zhang, Shenglei 2019-10-18 2:53 ` [edk2-devel] [PATCH] MdeModulePkg/HiiDatabaseDxe: Add check for 'Private->Attribute >> 4' Dandan Bi 0 siblings, 2 replies; 5+ messages in thread From: Zhang, Shenglei @ 2019-10-17 6:21 UTC (permalink / raw) To: devel; +Cc: Dandan Bi, Eric Dong The size of mHiiEfiColors is 16. mHiiEfiColors[Private->Attribute >> 4] may be out of boundary. So add a check for that. Cc: Dandan Bi <dandan.bi@intel.com> Cc: Eric Dong <eric.dong@intel.com> Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com> --- MdeModulePkg/Universal/HiiDatabaseDxe/Font.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/Font.c b/MdeModulePkg/Universal/HiiDatabaseDxe/Font.c index ca63df168c94..282a7a114d17 100644 --- a/MdeModulePkg/Universal/HiiDatabaseDxe/Font.c +++ b/MdeModulePkg/Universal/HiiDatabaseDxe/Font.c @@ -999,7 +999,12 @@ GetSystemFont ( } Info->ForegroundColor = mHiiEfiColors[Private->Attribute & 0x0f]; - Info->BackgroundColor = mHiiEfiColors[Private->Attribute >> 4]; + if ((Private->Attribute >> 4) < 16){ + Info->BackgroundColor = mHiiEfiColors[Private->Attribute >> 4]; + } else { + return EFI_INVALID_PARAMETER; + } + Info->FontInfoMask = EFI_FONT_INFO_SYS_FONT | EFI_FONT_INFO_SYS_SIZE | EFI_FONT_INFO_SYS_STYLE; Info->FontInfo.FontStyle = 0; Info->FontInfo.FontSize = EFI_GLYPH_HEIGHT; -- 2.18.0.windows.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH] MdeModulePkg/SdBlockIoPei: Add check for DeviceIndex 2019-10-17 6:21 [PATCH] MdeModulePkg/HiiDatabaseDxe: Add check for 'Private->Attribute >> 4' Zhang, Shenglei @ 2019-10-17 6:21 ` Zhang, Shenglei 2019-10-30 7:14 ` [edk2-devel] " Wu, Hao A 2019-10-18 2:53 ` [edk2-devel] [PATCH] MdeModulePkg/HiiDatabaseDxe: Add check for 'Private->Attribute >> 4' Dandan Bi 1 sibling, 1 reply; 5+ messages in thread From: Zhang, Shenglei @ 2019-10-17 6:21 UTC (permalink / raw) To: devel; +Cc: Hao A Wu, Ray Ni 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/SdBlockIoPei: Add check for DeviceIndex 2019-10-17 6:21 ` [PATCH] MdeModulePkg/SdBlockIoPei: Add check for DeviceIndex Zhang, Shenglei @ 2019-10-30 7:14 ` Wu, Hao A 2019-10-30 7:53 ` Zhang, Shenglei 0 siblings, 1 reply; 5+ messages in thread From: Wu, Hao A @ 2019-10-30 7:14 UTC (permalink / raw) To: devel@edk2.groups.io, Zhang, Shenglei; +Cc: Ni, Ray > -----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 > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/SdBlockIoPei: Add check for DeviceIndex 2019-10-30 7:14 ` [edk2-devel] " Wu, Hao A @ 2019-10-30 7:53 ` Zhang, Shenglei 0 siblings, 0 replies; 5+ messages in thread From: Zhang, Shenglei @ 2019-10-30 7:53 UTC (permalink / raw) To: Wu, Hao A, devel@edk2.groups.io; +Cc: Ni, Ray > -----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 > > > > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/HiiDatabaseDxe: Add check for 'Private->Attribute >> 4' 2019-10-17 6:21 [PATCH] MdeModulePkg/HiiDatabaseDxe: Add check for 'Private->Attribute >> 4' Zhang, Shenglei 2019-10-17 6:21 ` [PATCH] MdeModulePkg/SdBlockIoPei: Add check for DeviceIndex Zhang, Shenglei @ 2019-10-18 2:53 ` Dandan Bi 1 sibling, 0 replies; 5+ messages in thread From: Dandan Bi @ 2019-10-18 2:53 UTC (permalink / raw) To: devel@edk2.groups.io, Zhang, Shenglei; +Cc: Dong, Eric > -----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: Bi, Dandan <dandan.bi@intel.com>; Dong, Eric <eric.dong@intel.com> > Subject: [edk2-devel] [PATCH] MdeModulePkg/HiiDatabaseDxe: Add check > for 'Private->Attribute >> 4' > > The size of mHiiEfiColors is 16. > mHiiEfiColors[Private->Attribute >> 4] may be out of boundary. > So add a check for that. > > Cc: Dandan Bi <dandan.bi@intel.com> > Cc: Eric Dong <eric.dong@intel.com> > Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com> > --- > MdeModulePkg/Universal/HiiDatabaseDxe/Font.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/Font.c > b/MdeModulePkg/Universal/HiiDatabaseDxe/Font.c > index ca63df168c94..282a7a114d17 100644 > --- a/MdeModulePkg/Universal/HiiDatabaseDxe/Font.c > +++ b/MdeModulePkg/Universal/HiiDatabaseDxe/Font.c > @@ -999,7 +999,12 @@ GetSystemFont ( > } > > Info->ForegroundColor = mHiiEfiColors[Private->Attribute & 0x0f]; > - Info->BackgroundColor = mHiiEfiColors[Private->Attribute >> 4]; > + if ((Private->Attribute >> 4) < 16){ > + Info->BackgroundColor = mHiiEfiColors[Private->Attribute >> 4]; > + } else { > + return EFI_INVALID_PARAMETER; > + } > + Hi Shenglei, Foreground and background color are saved in a single byte. Bits 0..3 are the foreground color and bits 4..6 are the background color. If the Private->Attribute defined correctly, then (Private->Attribute >> 4) must less than 8. And in current code Private->Attribute is defined as EFI_TEXT_ATTR (EFI_LIGHTGRAY, EFI_BLACK) in HiiDatabaseEntry.c, so the (Private->Attribute >> 4) will not overflow. I think we can add ASSERT code instead. Thanks, Dandan > Info->FontInfoMask = EFI_FONT_INFO_SYS_FONT | > EFI_FONT_INFO_SYS_SIZE | EFI_FONT_INFO_SYS_STYLE; > Info->FontInfo.FontStyle = 0; > Info->FontInfo.FontSize = EFI_GLYPH_HEIGHT; > -- > 2.18.0.windows.1 > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-10-30 7:53 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-10-17 6:21 [PATCH] MdeModulePkg/HiiDatabaseDxe: Add check for 'Private->Attribute >> 4' Zhang, Shenglei 2019-10-17 6:21 ` [PATCH] MdeModulePkg/SdBlockIoPei: Add check for DeviceIndex Zhang, Shenglei 2019-10-30 7:14 ` [edk2-devel] " Wu, Hao A 2019-10-30 7:53 ` Zhang, Shenglei 2019-10-18 2:53 ` [edk2-devel] [PATCH] MdeModulePkg/HiiDatabaseDxe: Add check for 'Private->Attribute >> 4' Dandan Bi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox