* [edk2-staging][PATCH v2] BaseTools/Fmmt: Fix found the next FFS issue @ 2020-05-09 7:02 Feng, YunhuaX 2020-05-09 8:28 ` Ni, Ray 0 siblings, 1 reply; 3+ messages in thread From: Feng, YunhuaX @ 2020-05-09 7:02 UTC (permalink / raw) To: devel@edk2.groups.io; +Cc: Ni, Ray, Gao, Liming, Feng, Bob C if the ffs size is invalid, break the iteration and return NOT_FOUND. Cc: Ray Ni< ray.ni@intel.com> Cc: Bob Feng <bob.c.feng@intel.com> Cc: Liming Gao <liming.gao@intel.com> Change-Id: I9e320d6176af350ff208901209f3f6c89e4e1924 Signed-off-by: Yunhua Feng <yunhuax.feng@intel.com> --- BaseTools/Source/C/FMMT/FmmtLib.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/BaseTools/Source/C/FMMT/FmmtLib.c b/BaseTools/Source/C/FMMT/FmmtLib.c index 9ec511ef06..6858cfe778 100644 --- a/BaseTools/Source/C/FMMT/FmmtLib.c +++ b/BaseTools/Source/C/FMMT/FmmtLib.c @@ -1769,10 +1769,17 @@ FvBufFindNextFile ( (*Key + sizeof (*fhdr)) < fvSize; *Key = (UINTN)ALIGN_POINTER (*Key, 8) ) { fhdr = (EFI_FFS_FILE_HEADER*) ((UINT8*)hdr + *Key); fsize = GetFfsFileLength (fhdr); + // + //if the ffs size is invalid, break the loop + //the size defined in EFI_FFS_FILE_HEADER is 3 bytes. + // + if (fsize == (UINTN)((FvbAttributes & EFI_FVB2_ERASE_POLARITY) ? 0xFFFFFF : 0x0)) { + break; + } if (!EFI_TEST_FFS_ATTRIBUTES_BIT( FvbAttributes, fhdr->State, EFI_FILE_HEADER_VALID ) || -- 2.12.2.windows.2 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [edk2-staging][PATCH v2] BaseTools/Fmmt: Fix found the next FFS issue 2020-05-09 7:02 [edk2-staging][PATCH v2] BaseTools/Fmmt: Fix found the next FFS issue Feng, YunhuaX @ 2020-05-09 8:28 ` Ni, Ray 2020-05-09 9:15 ` Feng, YunhuaX 0 siblings, 1 reply; 3+ messages in thread From: Ni, Ray @ 2020-05-09 8:28 UTC (permalink / raw) To: Feng, YunhuaX, devel@edk2.groups.io; +Cc: Gao, Liming, Feng, Bob C Yunhua, I understand that if the ffs size 0 should be specially handled because otherwise the for-loop will be infinite. But why do you need to check the size against 0xFFFFFF? Why the break condition in for-loop " (*Key + sizeof (*fhdr)) < fvSize;" cannot help when the fsize is too big such as 0xFFFFFF? I suggest you explain what issue you want to fix and how this fixes the issue in commit message in detail. Thanks, Ray > -----Original Message----- > From: Feng, YunhuaX <yunhuax.feng@intel.com> > Sent: Saturday, May 9, 2020 3:03 PM > To: devel@edk2.groups.io > Cc: Ni, Ray <ray.ni@intel.com>; Gao, Liming <liming.gao@intel.com>; Feng, Bob C <bob.c.feng@intel.com> > Subject: [edk2-staging][PATCH v2] BaseTools/Fmmt: Fix found the next FFS issue > > if the ffs size is invalid, break the iteration and return NOT_FOUND. > > Cc: Ray Ni< ray.ni@intel.com> > Cc: Bob Feng <bob.c.feng@intel.com> > Cc: Liming Gao <liming.gao@intel.com> > > Change-Id: I9e320d6176af350ff208901209f3f6c89e4e1924 > Signed-off-by: Yunhua Feng <yunhuax.feng@intel.com> > --- > BaseTools/Source/C/FMMT/FmmtLib.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/BaseTools/Source/C/FMMT/FmmtLib.c b/BaseTools/Source/C/FMMT/FmmtLib.c > index 9ec511ef06..6858cfe778 100644 > --- a/BaseTools/Source/C/FMMT/FmmtLib.c > +++ b/BaseTools/Source/C/FMMT/FmmtLib.c > @@ -1769,10 +1769,17 @@ FvBufFindNextFile ( > (*Key + sizeof (*fhdr)) < fvSize; > *Key = (UINTN)ALIGN_POINTER (*Key, 8) > ) { > fhdr = (EFI_FFS_FILE_HEADER*) ((UINT8*)hdr + *Key); > fsize = GetFfsFileLength (fhdr); > + // > + //if the ffs size is invalid, break the loop > + //the size defined in EFI_FFS_FILE_HEADER is 3 bytes. > + // > + if (fsize == (UINTN)((FvbAttributes & EFI_FVB2_ERASE_POLARITY) ? 0xFFFFFF : 0x0)) { > + break; > + } > if (!EFI_TEST_FFS_ATTRIBUTES_BIT( > FvbAttributes, > fhdr->State, > EFI_FILE_HEADER_VALID > ) || > -- > 2.12.2.windows.2 > ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [edk2-staging][PATCH v2] BaseTools/Fmmt: Fix found the next FFS issue 2020-05-09 8:28 ` Ni, Ray @ 2020-05-09 9:15 ` Feng, YunhuaX 0 siblings, 0 replies; 3+ messages in thread From: Feng, YunhuaX @ 2020-05-09 9:15 UTC (permalink / raw) To: Ni, Ray, devel@edk2.groups.io; +Cc: Gao, Liming, Feng, Bob C Hi Ray, I encounter a FV image, behind valid FFS is filled with 0xff, and then filled with some 0x00. In the for-loop, When the fsize == 0xFFFFFF, *key = *key + 1, so not break. when the fsize == 0x0, *key = *key + fsize, so it is dead loop. I hope that if ffs size invalid, break the for-loop. -----Original Message----- From: Ni, Ray <ray.ni@intel.com> Sent: Saturday, May 9, 2020 4:28 PM To: Feng, YunhuaX <yunhuax.feng@intel.com>; devel@edk2.groups.io Cc: Gao, Liming <liming.gao@intel.com>; Feng, Bob C <bob.c.feng@intel.com> Subject: RE: [edk2-staging][PATCH v2] BaseTools/Fmmt: Fix found the next FFS issue Yunhua, I understand that if the ffs size 0 should be specially handled because otherwise the for-loop will be infinite. But why do you need to check the size against 0xFFFFFF? Why the break condition in for-loop " (*Key + sizeof (*fhdr)) < fvSize;" cannot help when the fsize is too big such as 0xFFFFFF? I suggest you explain what issue you want to fix and how this fixes the issue in commit message in detail. Thanks, Ray > -----Original Message----- > From: Feng, YunhuaX <yunhuax.feng@intel.com> > Sent: Saturday, May 9, 2020 3:03 PM > To: devel@edk2.groups.io > Cc: Ni, Ray <ray.ni@intel.com>; Gao, Liming <liming.gao@intel.com>; > Feng, Bob C <bob.c.feng@intel.com> > Subject: [edk2-staging][PATCH v2] BaseTools/Fmmt: Fix found the next > FFS issue > > if the ffs size is invalid, break the iteration and return NOT_FOUND. > > Cc: Ray Ni< ray.ni@intel.com> > Cc: Bob Feng <bob.c.feng@intel.com> > Cc: Liming Gao <liming.gao@intel.com> > > Change-Id: I9e320d6176af350ff208901209f3f6c89e4e1924 > Signed-off-by: Yunhua Feng <yunhuax.feng@intel.com> > --- > BaseTools/Source/C/FMMT/FmmtLib.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/BaseTools/Source/C/FMMT/FmmtLib.c > b/BaseTools/Source/C/FMMT/FmmtLib.c > index 9ec511ef06..6858cfe778 100644 > --- a/BaseTools/Source/C/FMMT/FmmtLib.c > +++ b/BaseTools/Source/C/FMMT/FmmtLib.c > @@ -1769,10 +1769,17 @@ FvBufFindNextFile ( > (*Key + sizeof (*fhdr)) < fvSize; > *Key = (UINTN)ALIGN_POINTER (*Key, 8) > ) { > fhdr = (EFI_FFS_FILE_HEADER*) ((UINT8*)hdr + *Key); > fsize = GetFfsFileLength (fhdr); > + // > + //if the ffs size is invalid, break the loop > + //the size defined in EFI_FFS_FILE_HEADER is 3 bytes. > + // > + if (fsize == (UINTN)((FvbAttributes & EFI_FVB2_ERASE_POLARITY) ? 0xFFFFFF : 0x0)) { > + break; > + } > if (!EFI_TEST_FFS_ATTRIBUTES_BIT( > FvbAttributes, > fhdr->State, > EFI_FILE_HEADER_VALID > ) || > -- > 2.12.2.windows.2 > ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-05-09 9:15 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-05-09 7:02 [edk2-staging][PATCH v2] BaseTools/Fmmt: Fix found the next FFS issue Feng, YunhuaX 2020-05-09 8:28 ` Ni, Ray 2020-05-09 9:15 ` Feng, YunhuaX
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox