* [PATCH] ShellPkg/Pci: Add valid check for PCI extended config space parser @ 2021-04-10 14:15 IanX Kuo 2021-03-17 21:00 ` [edk2-devel] " Laszlo Ersek 0 siblings, 1 reply; 3+ messages in thread From: IanX Kuo @ 2021-04-10 14:15 UTC (permalink / raw) To: devel; +Cc: VincentX Ke From: VincentX Ke <vincentx.ke@intel.com> Bugzilla: 3262 (https://bugzilla.tianocore.org/show_bug.cgi?id=3262) No need to print PCIe details while CapabilityId is 0xFFFF. Limit the NextCapabilityOffset to PCI configuration space. Signed-off-by: VincentX Ke <vincentx.ke@intel.com> --- ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c b/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c index a2f04d8db5..1e5dc75e27 100644 --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c @@ -2038,12 +2038,14 @@ LocatePciCapability ( @param[in] PciExpressCap PCI Express capability buffer. @param[in] ExtendedConfigSpace PCI Express extended configuration space. + @param[in] ExtendedConfigSize PCI Express extended configuration size. @param[in] ExtendedCapability PCI Express extended capability ID to explain. **/ VOID PciExplainPciExpress ( IN PCI_CAPABILITY_PCIEXP *PciExpressCap, IN UINT8 *ExtendedConfigSpace, + IN UINTN ExtendedConfigSize, IN CONST UINT16 ExtendedCapability ); @@ -2921,6 +2923,7 @@ ShellCommandRunPci ( PciExplainPciExpress ( (PCI_CAPABILITY_PCIEXP *) ((UINT8 *) &ConfigSpace + PcieCapabilityPtr), ExtendedConfigSpace, + ExtendedConfigSize, ExtendedCapability ); } @@ -5698,12 +5701,14 @@ PrintPciExtendedCapabilityDetails( @param[in] PciExpressCap PCI Express capability buffer. @param[in] ExtendedConfigSpace PCI Express extended configuration space. + @param[in] ExtendedConfigSize PCI Express extended configuration size. @param[in] ExtendedCapability PCI Express extended capability ID to explain. **/ VOID PciExplainPciExpress ( IN PCI_CAPABILITY_PCIEXP *PciExpressCap, IN UINT8 *ExtendedConfigSpace, + IN UINTN ExtendedConfigSize, IN CONST UINT16 ExtendedCapability ) { @@ -5786,7 +5791,7 @@ PciExplainPciExpress ( } ExtHdr = (PCI_EXP_EXT_HDR*)ExtendedConfigSpace; - while (ExtHdr->CapabilityId != 0 && ExtHdr->CapabilityVersion != 0) { + while (ExtHdr->CapabilityId != 0 && ExtHdr->CapabilityVersion != 0 && ExtHdr->CapabilityId != 0xFFFF) { // // Process this item // @@ -5800,7 +5805,8 @@ PciExplainPciExpress ( // // Advance to the next item if it exists // - if (ExtHdr->NextCapabilityOffset != 0) { + if (ExtHdr->NextCapabilityOffset != 0 && + (ExtHdr->NextCapabilityOffset <= (UINT32) (ExtendedConfigSize + EFI_PCIE_CAPABILITY_BASE_OFFSET - sizeof (PCI_EXP_EXT_HDR)))) { ExtHdr = (PCI_EXP_EXT_HDR*)(ExtendedConfigSpace + ExtHdr->NextCapabilityOffset - EFI_PCIE_CAPABILITY_BASE_OFFSET); } else { break; -- 2.18.0.windows.1 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [edk2-devel] [PATCH] ShellPkg/Pci: Add valid check for PCI extended config space parser 2021-04-10 14:15 [PATCH] ShellPkg/Pci: Add valid check for PCI extended config space parser IanX Kuo @ 2021-03-17 21:00 ` Laszlo Ersek 2021-03-17 21:29 ` IanX Kuo 0 siblings, 1 reply; 3+ messages in thread From: Laszlo Ersek @ 2021-03-17 21:00 UTC (permalink / raw) To: devel, ianx.kuo; +Cc: VincentX Ke Vincent, Ian: something is seriously broken in your email setup. I have seen three messages from you guys on the list, and each one of those is "from the future". Here are the Date headers from the messages: - Date: Thu, 8 Apr 2021 05:50:21 +0800 https://edk2.groups.io/g/devel/message/72748 - Date: Sat, 10 Apr 2021 01:34:45 +0800 https://edk2.groups.io/g/devel/message/72862 - Date: Sat, 10 Apr 2021 22:15:09 +0800 https://edk2.groups.io/g/devel/message/72953 Please fix your clock setup, and post a new version. This is why I am asking: the git-am manual says, --ignore-date By default the command records the date from the e-mail message as the commit author date, and uses the time of commit creation as the committer date. [...] When we merge a patch using a github.com Pull Request, the "mergify bot" preserves the Author Date field. That's a good thing in itself, but it means that the edk2 commit history would have a patch from the future -- a patch committed in March, but "authored" in April. That's bogus. Whoever actually applies the patch (probably the ShellPkg maintainer) can work around the issue, by specifying the "--ignore-date" flag for "git-am". But that's easy to forget, and not the right thing anyway. So please just fix your broken clock, and post a new version. Also: you should have used v1, v2, v3 in the subject prefixes (just pass -v1, -v2, -v3 to git-format-patch). The next version that you post should be marked "v4". Thanks Laszlo On 04/10/21 16:15, IanX Kuo wrote: > From: VincentX Ke <vincentx.ke@intel.com> > > Bugzilla: 3262 (https://bugzilla.tianocore.org/show_bug.cgi?id=3262) > > No need to print PCIe details while CapabilityId is 0xFFFF. > Limit the NextCapabilityOffset to PCI configuration space. > > Signed-off-by: VincentX Ke <vincentx.ke@intel.com> > --- > ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c b/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c > index a2f04d8db5..1e5dc75e27 100644 > --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c > +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c > @@ -2038,12 +2038,14 @@ LocatePciCapability ( > > @param[in] PciExpressCap PCI Express capability buffer. > @param[in] ExtendedConfigSpace PCI Express extended configuration space. > + @param[in] ExtendedConfigSize PCI Express extended configuration size. > @param[in] ExtendedCapability PCI Express extended capability ID to explain. > **/ > VOID > PciExplainPciExpress ( > IN PCI_CAPABILITY_PCIEXP *PciExpressCap, > IN UINT8 *ExtendedConfigSpace, > + IN UINTN ExtendedConfigSize, > IN CONST UINT16 ExtendedCapability > ); > > @@ -2921,6 +2923,7 @@ ShellCommandRunPci ( > PciExplainPciExpress ( > (PCI_CAPABILITY_PCIEXP *) ((UINT8 *) &ConfigSpace + PcieCapabilityPtr), > ExtendedConfigSpace, > + ExtendedConfigSize, > ExtendedCapability > ); > } > @@ -5698,12 +5701,14 @@ PrintPciExtendedCapabilityDetails( > > @param[in] PciExpressCap PCI Express capability buffer. > @param[in] ExtendedConfigSpace PCI Express extended configuration space. > + @param[in] ExtendedConfigSize PCI Express extended configuration size. > @param[in] ExtendedCapability PCI Express extended capability ID to explain. > **/ > VOID > PciExplainPciExpress ( > IN PCI_CAPABILITY_PCIEXP *PciExpressCap, > IN UINT8 *ExtendedConfigSpace, > + IN UINTN ExtendedConfigSize, > IN CONST UINT16 ExtendedCapability > ) > { > @@ -5786,7 +5791,7 @@ PciExplainPciExpress ( > } > > ExtHdr = (PCI_EXP_EXT_HDR*)ExtendedConfigSpace; > - while (ExtHdr->CapabilityId != 0 && ExtHdr->CapabilityVersion != 0) { > + while (ExtHdr->CapabilityId != 0 && ExtHdr->CapabilityVersion != 0 && ExtHdr->CapabilityId != 0xFFFF) { > // > // Process this item > // > @@ -5800,7 +5805,8 @@ PciExplainPciExpress ( > // > // Advance to the next item if it exists > // > - if (ExtHdr->NextCapabilityOffset != 0) { > + if (ExtHdr->NextCapabilityOffset != 0 && > + (ExtHdr->NextCapabilityOffset <= (UINT32) (ExtendedConfigSize + EFI_PCIE_CAPABILITY_BASE_OFFSET - sizeof (PCI_EXP_EXT_HDR)))) { > ExtHdr = (PCI_EXP_EXT_HDR*)(ExtendedConfigSpace + ExtHdr->NextCapabilityOffset - EFI_PCIE_CAPABILITY_BASE_OFFSET); > } else { > break; > ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [edk2-devel] [PATCH] ShellPkg/Pci: Add valid check for PCI extended config space parser 2021-03-17 21:00 ` [edk2-devel] " Laszlo Ersek @ 2021-03-17 21:29 ` IanX Kuo 0 siblings, 0 replies; 3+ messages in thread From: IanX Kuo @ 2021-03-17 21:29 UTC (permalink / raw) To: Laszlo Ersek, devel@edk2.groups.io; +Cc: Ke, VincentX Hi Laszlo Thanks for the remind. I will take care the date and Patch v4 in our next patch. Thanks, Ian Kuo -----Original Message----- From: Laszlo Ersek <lersek@redhat.com> Sent: Thursday, March 18, 2021 5:01 AM To: devel@edk2.groups.io; Kuo, IanX <ianx.kuo@intel.com> Cc: Ke, VincentX <vincentx.ke@intel.com> Subject: Re: [edk2-devel] [PATCH] ShellPkg/Pci: Add valid check for PCI extended config space parser Vincent, Ian: something is seriously broken in your email setup. I have seen three messages from you guys on the list, and each one of those is "from the future". Here are the Date headers from the messages: - Date: Thu, 8 Apr 2021 05:50:21 +0800 https://edk2.groups.io/g/devel/message/72748 - Date: Sat, 10 Apr 2021 01:34:45 +0800 https://edk2.groups.io/g/devel/message/72862 - Date: Sat, 10 Apr 2021 22:15:09 +0800 https://edk2.groups.io/g/devel/message/72953 Please fix your clock setup, and post a new version. This is why I am asking: the git-am manual says, --ignore-date By default the command records the date from the e-mail message as the commit author date, and uses the time of commit creation as the committer date. [...] When we merge a patch using a github.com Pull Request, the "mergify bot" preserves the Author Date field. That's a good thing in itself, but it means that the edk2 commit history would have a patch from the future -- a patch committed in March, but "authored" in April. That's bogus. Whoever actually applies the patch (probably the ShellPkg maintainer) can work around the issue, by specifying the "--ignore-date" flag for "git-am". But that's easy to forget, and not the right thing anyway. So please just fix your broken clock, and post a new version. Also: you should have used v1, v2, v3 in the subject prefixes (just pass -v1, -v2, -v3 to git-format-patch). The next version that you post should be marked "v4". Thanks Laszlo On 04/10/21 16:15, IanX Kuo wrote: > From: VincentX Ke <vincentx.ke@intel.com> > > Bugzilla: 3262 (https://bugzilla.tianocore.org/show_bug.cgi?id=3262) > > No need to print PCIe details while CapabilityId is 0xFFFF. > Limit the NextCapabilityOffset to PCI configuration space. > > Signed-off-by: VincentX Ke <vincentx.ke@intel.com> > --- > ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c > b/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c > index a2f04d8db5..1e5dc75e27 100644 > --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c > +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c > @@ -2038,12 +2038,14 @@ LocatePciCapability ( > > @param[in] PciExpressCap PCI Express capability buffer. > @param[in] ExtendedConfigSpace PCI Express extended configuration space. > + @param[in] ExtendedConfigSize PCI Express extended configuration size. > @param[in] ExtendedCapability PCI Express extended capability ID to explain. > **/ > VOID > PciExplainPciExpress ( > IN PCI_CAPABILITY_PCIEXP *PciExpressCap, > IN UINT8 *ExtendedConfigSpace, > + IN UINTN ExtendedConfigSize, > IN CONST UINT16 ExtendedCapability > ); > > @@ -2921,6 +2923,7 @@ ShellCommandRunPci ( > PciExplainPciExpress ( > (PCI_CAPABILITY_PCIEXP *) ((UINT8 *) &ConfigSpace + PcieCapabilityPtr), > ExtendedConfigSpace, > + ExtendedConfigSize, > ExtendedCapability > ); > } > @@ -5698,12 +5701,14 @@ PrintPciExtendedCapabilityDetails( > > @param[in] PciExpressCap PCI Express capability buffer. > @param[in] ExtendedConfigSpace PCI Express extended configuration space. > + @param[in] ExtendedConfigSize PCI Express extended configuration size. > @param[in] ExtendedCapability PCI Express extended capability ID to explain. > **/ > VOID > PciExplainPciExpress ( > IN PCI_CAPABILITY_PCIEXP *PciExpressCap, > IN UINT8 *ExtendedConfigSpace, > + IN UINTN ExtendedConfigSize, > IN CONST UINT16 ExtendedCapability > ) > { > @@ -5786,7 +5791,7 @@ PciExplainPciExpress ( > } > > ExtHdr = (PCI_EXP_EXT_HDR*)ExtendedConfigSpace; > - while (ExtHdr->CapabilityId != 0 && ExtHdr->CapabilityVersion != 0) > { > + while (ExtHdr->CapabilityId != 0 && ExtHdr->CapabilityVersion != 0 > + && ExtHdr->CapabilityId != 0xFFFF) { > // > // Process this item > // > @@ -5800,7 +5805,8 @@ PciExplainPciExpress ( > // > // Advance to the next item if it exists > // > - if (ExtHdr->NextCapabilityOffset != 0) { > + if (ExtHdr->NextCapabilityOffset != 0 && > + (ExtHdr->NextCapabilityOffset <= (UINT32) (ExtendedConfigSize > + + EFI_PCIE_CAPABILITY_BASE_OFFSET - sizeof (PCI_EXP_EXT_HDR)))) { > ExtHdr = (PCI_EXP_EXT_HDR*)(ExtendedConfigSpace + ExtHdr->NextCapabilityOffset - EFI_PCIE_CAPABILITY_BASE_OFFSET); > } else { > break; > ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-03-17 21:29 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-04-10 14:15 [PATCH] ShellPkg/Pci: Add valid check for PCI extended config space parser IanX Kuo 2021-03-17 21:00 ` [edk2-devel] " Laszlo Ersek 2021-03-17 21:29 ` IanX Kuo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox