* 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
* [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
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