Re: [edk2] [PATCH v2 2/2] MdePkg/UefiDevicePathLib: Add a checking step
Reviewed-by: Ruiyu Ni > -Original Message- > From: edk2-devel On Behalf Of Shenglei > Zhang > Sent: Wednesday, December 12, 2018 11:10 AM > To: edk2-devel@lists.01.org > Cc: Kinney, Michael D ; Gao, Liming > > Subject: [edk2] [PATCH v2 2/2] MdePkg/UefiDevicePathLib: Add a checking > step > > Add a checking step in DevicePathUtilities.c to verify the DevicePath. > https://bugzilla.tianocore.org/show_bug.cgi?id=1372 > > v2: Remove ASSERT() and the redundant checking step. > Update related description in DevicePathLib.h > > Cc: Liming Gao > Cc: Michael D Kinney > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Shenglei Zhang > --- > MdePkg/Include/Library/DevicePathLib.h | 2 +- > .../UefiDevicePathLib/DevicePathUtilities.c| 14 ++ > 2 files changed, 7 insertions(+), 9 deletions(-) > > diff --git a/MdePkg/Include/Library/DevicePathLib.h > b/MdePkg/Include/Library/DevicePathLib.h > index 959299704a..717d4db42f 100644 > --- a/MdePkg/Include/Library/DevicePathLib.h > +++ b/MdePkg/Include/Library/DevicePathLib.h > @@ -22,7 +22,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY > KIND, EITHER EXPRESS OR IMPLIED. > > /** >Determine whether a given device path is valid. > - If DevicePath is NULL, then ASSERT(). > + If DevicePath is NULL or the size is not suitable, then return false. > >@param DevicePath A pointer to a device path data structure. >@param MaxSize The maximum size of the device path data structure. > diff --git a/MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c > b/MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c > index 665e5a4adc..05f82c1313 100644 > --- a/MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c > +++ b/MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c > @@ -59,19 +59,17 @@ IsDevicePathValid ( >UINTN Size; >UINTN NodeLength; > > - ASSERT (DevicePath != NULL); > - > - if (MaxSize == 0) { > -MaxSize = MAX_UINTN; > - } > - >// > - // Validate the input size big enough to touch the first node. > + //Validate the input whether exists and its size big enough to touch the > first node >// > - if (MaxSize < sizeof (EFI_DEVICE_PATH_PROTOCOL)) { > + if (DevicePath == NULL || (MaxSize > 0 && MaxSize < > END_DEVICE_PATH_LENGTH)) { > return FALSE; >} > > + if (MaxSize == 0) { > +MaxSize = MAX_UINTN; > + } > + >for (Count = 0, Size = 0; !IsDevicePathEnd (DevicePath); DevicePath = > NextDevicePathNode (DevicePath)) { > NodeLength = DevicePathNodeLength (DevicePath); > if (NodeLength < sizeof (EFI_DEVICE_PATH_PROTOCOL)) { > -- > 2.18.0.windows.1 > > ___ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 2/2] MdePkg/UefiDevicePathLib: Add a checking step
Shenglei: The patch is good. Reviewed-by: Liming Gao Besides, this patch is from MS Mu project. Please keep original patch author. You don't need to send this patch again. Thanks Liming > -Original Message- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > Shenglei Zhang > Sent: Wednesday, December 12, 2018 11:10 AM > To: edk2-devel@lists.01.org > Cc: Kinney, Michael D ; Gao, Liming > > Subject: [edk2] [PATCH v2 2/2] MdePkg/UefiDevicePathLib: Add a checking step > > Add a checking step in DevicePathUtilities.c to verify the DevicePath. > https://bugzilla.tianocore.org/show_bug.cgi?id=1372 > > v2: Remove ASSERT() and the redundant checking step. > Update related description in DevicePathLib.h > > Cc: Liming Gao > Cc: Michael D Kinney > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Shenglei Zhang > --- > MdePkg/Include/Library/DevicePathLib.h | 2 +- > .../UefiDevicePathLib/DevicePathUtilities.c| 14 ++ > 2 files changed, 7 insertions(+), 9 deletions(-) > > diff --git a/MdePkg/Include/Library/DevicePathLib.h > b/MdePkg/Include/Library/DevicePathLib.h > index 959299704a..717d4db42f 100644 > --- a/MdePkg/Include/Library/DevicePathLib.h > +++ b/MdePkg/Include/Library/DevicePathLib.h > @@ -22,7 +22,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER > EXPRESS OR IMPLIED. > > /** >Determine whether a given device path is valid. > - If DevicePath is NULL, then ASSERT(). > + If DevicePath is NULL or the size is not suitable, then return false. > >@param DevicePath A pointer to a device path data structure. >@param MaxSize The maximum size of the device path data structure. > diff --git a/MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c > b/MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c > index 665e5a4adc..05f82c1313 100644 > --- a/MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c > +++ b/MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c > @@ -59,19 +59,17 @@ IsDevicePathValid ( >UINTN Size; >UINTN NodeLength; > > - ASSERT (DevicePath != NULL); > - > - if (MaxSize == 0) { > -MaxSize = MAX_UINTN; > - } > - >// > - // Validate the input size big enough to touch the first node. > + //Validate the input whether exists and its size big enough to touch the > first node >// > - if (MaxSize < sizeof (EFI_DEVICE_PATH_PROTOCOL)) { > + if (DevicePath == NULL || (MaxSize > 0 && MaxSize < > END_DEVICE_PATH_LENGTH)) { > return FALSE; >} > > + if (MaxSize == 0) { > +MaxSize = MAX_UINTN; > + } > + >for (Count = 0, Size = 0; !IsDevicePathEnd (DevicePath); DevicePath = > NextDevicePathNode (DevicePath)) { > NodeLength = DevicePathNodeLength (DevicePath); > if (NodeLength < sizeof (EFI_DEVICE_PATH_PROTOCOL)) { > -- > 2.18.0.windows.1 > > ___ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 2/2] MdePkg/UefiDevicePathLib: Add a checking step
Add a checking step in DevicePathUtilities.c to verify the DevicePath. https://bugzilla.tianocore.org/show_bug.cgi?id=1372 v2: Remove ASSERT() and the redundant checking step. Update related description in DevicePathLib.h Cc: Liming Gao Cc: Michael D Kinney Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Shenglei Zhang --- MdePkg/Include/Library/DevicePathLib.h | 2 +- .../UefiDevicePathLib/DevicePathUtilities.c| 14 ++ 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/MdePkg/Include/Library/DevicePathLib.h b/MdePkg/Include/Library/DevicePathLib.h index 959299704a..717d4db42f 100644 --- a/MdePkg/Include/Library/DevicePathLib.h +++ b/MdePkg/Include/Library/DevicePathLib.h @@ -22,7 +22,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. /** Determine whether a given device path is valid. - If DevicePath is NULL, then ASSERT(). + If DevicePath is NULL or the size is not suitable, then return false. @param DevicePath A pointer to a device path data structure. @param MaxSize The maximum size of the device path data structure. diff --git a/MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c b/MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c index 665e5a4adc..05f82c1313 100644 --- a/MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c +++ b/MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c @@ -59,19 +59,17 @@ IsDevicePathValid ( UINTN Size; UINTN NodeLength; - ASSERT (DevicePath != NULL); - - if (MaxSize == 0) { -MaxSize = MAX_UINTN; - } - // - // Validate the input size big enough to touch the first node. + //Validate the input whether exists and its size big enough to touch the first node // - if (MaxSize < sizeof (EFI_DEVICE_PATH_PROTOCOL)) { + if (DevicePath == NULL || (MaxSize > 0 && MaxSize < END_DEVICE_PATH_LENGTH)) { return FALSE; } + if (MaxSize == 0) { +MaxSize = MAX_UINTN; + } + for (Count = 0, Size = 0; !IsDevicePathEnd (DevicePath); DevicePath = NextDevicePathNode (DevicePath)) { NodeLength = DevicePathNodeLength (DevicePath); if (NodeLength < sizeof (EFI_DEVICE_PATH_PROTOCOL)) { -- 2.18.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel