IB/hfi1: Check the return value of pcie_capability_read_*()
Subsystems (9)
PATCH v3 1/14 IB/hfi1: Check the return value of pcie_capability_read_*()
From: Bolarinwa Olayemi Saheed <[email protected]>
On failure pcie_capability_read_dword() sets it's last parameter, val to 0. In this case dn and up will be 0, so aspm_hw_l1_supported() will return false.
However, with Patch 14/14, it is possible that val is set to ~0 on failure. This would introduce a bug because (x & x) == (~0 & x). So with dn and up being 0x02, a true value is return when the read has actually failed.
This bug can be avoided if the return value of pcie_capability_read_dword is checked to confirm success. The behaviour of the function remains intact.
Check the return value of pcie_capability_read_dword() to ensure success.
--- drivers/infiniband/hw/hfi1/aspm.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
drivers/infiniband/hw/hfi1/aspm.c +4 -3
@@ -24,6 +24,7 @@ static bool aspm_hw_l1_supported(struct hfi1_devdata *dd) {struct pci_dev *parent = dd->pcidev->bus->self;u32 up, dn; +int ret_up, ret_dn;/* * If the driver does not have access to the upstream component,
@@ -32,14 +33,14 @@ static bool aspm_hw_l1_supported(struct hfi1_devdata *dd)if (!parent)return false; -pcie_capability_read_dword (dd->pcidev,PCI_EXP_LNKCAP , &dn); + ret_dn =pcie_capability_read_dword (dd->pcidev,PCI_EXP_LNKCAP , &dn); dn =ASPM_L1_SUPPORTED (dn); -pcie_capability_read_dword (parent,PCI_EXP_LNKCAP , &up); + ret_up =pcie_capability_read_dword (parent,PCI_EXP_LNKCAP , &up); up =ASPM_L1_SUPPORTED (up);/* ASPM works on A-step but is reported as not supported */ -return (!!dn ||is_ax (dd)) && !!up; +return !!ret_dn && !!ret_up && (!!dn ||is_ax (dd)) && !!up; }/* Set L1 entrance latency for slower entry to L1 */
Signature · 1 line
2.18.2
PATCH v3 3/14 ath9k: Check the return value of pcie_capability_read_*()
From: Bolarinwa Olayemi Saheed <[email protected]>
On failure pcie_capability_read_dword() sets it's last parameter, val to 0. However, with Patch 14/14, it is possible that val is set to ~0 on failure. This would introduce a bug because (x & x) == (~0 & x).
This bug can be avoided without changing the function's behaviour if the return value of pcie_capability_read_dword is checked to confirm success.
Check the return value of pcie_capability_read_dword() to ensure success.
--- drivers/net/wireless/ath/ath9k/pci.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/net/wireless/ath/ath9k/pci.c +3 -2
@@ -825,6 +825,7 @@ static void ath_pci_aspm_init(struct ath_common *common)struct pci_dev *pdev =to_pci_dev (sc->dev);struct pci_dev *parent;u16 aspm; +int ret;if (!ah->is_pciexpress)return ;
@@ -866,8 +867,8 @@ static void ath_pci_aspm_init(struct ath_common *common)if (AR_SREV_9462 (ah))pci_read_config_dword (pdev,0x70c , &ah->config.aspm_l1_fix); -pcie_capability_read_word (parent,PCI_EXP_LNKCTL , &aspm); -if (aspm & (PCI_EXP_LNKCTL_ASPM_L0S |PCI_EXP_LNKCTL_ASPM_L1 )) { + ret =pcie_capability_read_word (parent,PCI_EXP_LNKCTL , &aspm); +if (!ret && (aspm & (PCI_EXP_LNKCTL_ASPM_L0S |PCI_EXP_LNKCTL_ASPM_L1 ))) { ah->aspm_enabled = true;/* Initialize PCIe PM and SERDES registers. */ ath9k_hw_configpcipowersave (ah, false);
Signature · 1 line
2.18.2
Re: [PATCH 3/14 v3] ath9k: Check the return value of pcie_capability_read_*()
Quoted 4 lines · Saheed O. Bolarinwa
On failure
pcie_capability_read_dword()sets it's last parameter, val to 0. However, with Patch 14/14, it is possible that val is set to ~0 on failure. This would introduce a bug because (x & x) == (~0 & x).
Quoted 8 lines
This bug can be avoided without changing the function's behaviour if the return value of
pcie_capability_read_dwordis checked to confirm success.Check the return value of
pcie_capability_read_dword()to ensure success.Suggested-by Bjorn Helgaas <[email protected]>Signed-off-by Bolarinwa Olayemi Saheed <[email protected]>Signed-off-by Kalle Valo <[email protected]>
Patch applied to ath-next branch of ath.git, thanks.
9a8ab2bfb678 ath9k: Check the return value of pcie_capability_read_*()
Signature · 3 lines
https://patchwork.kernel.org/patch/11660731/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [Linux-kernel-mentees] [PATCH 3/14 v3] ath9k: Check the return value of pcie_capability_read_*()
In reply to Saheed O. BolarinwaQuoted 4 lines · Saheed O. Bolarinwa
On failure
pcie_capability_read_dword()sets it's last parameter, val to 0. However, with Patch 14/14, it is possible that val is set to ~0 on failure. This would introduce a bug because (x & x) == (~0 & x).
Quoted 8 lines
This bug can be avoided without changing the function's behaviour if the return value of
pcie_capability_read_dwordis checked to confirm success.Check the return value of
pcie_capability_read_dword()to ensure success.Suggested-by Bjorn Helgaas <[email protected]>Signed-off-by Bolarinwa Olayemi Saheed <[email protected]>Signed-off-by Kalle Valo <[email protected]>
Patch applied to ath-next branch of ath.git, thanks.
9a8ab2bfb678 ath9k: Check the return value of pcie_capability_read_*()
Signature · 8 lines
https://patchwork.kernel.org/patch/11660731/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
PATCH v3 4/14 iwlegacy: Check the return value of pcie_capability_read_*()
In reply to Saheed O. BolarinwaFrom: Bolarinwa Olayemi Saheed <[email protected]>
On failure pcie_capability_read_dword() sets it's last parameter, val to 0. However, with Patch 14/14, it is possible that val is set to ~0 on failure. This would introduce a bug because (x & x) == (~0 & x).
This bug can be avoided without changing the function's behaviour if the return value of pcie_capability_read_dword is checked to confirm success.
Check the return value of pcie_capability_read_dword() to ensure success.
--- drivers/net/wireless/intel/iwlegacy/common.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/net/wireless/intel/iwlegacy/common.c +2 -2
@@ -4286,8 +4286,8 @@ il_apm_init(struct il_priv *il)* power savings, even without L1. */ if (il->cfg->set_l0s) { -pcie_capability_read_word (il->pci_dev,PCI_EXP_LNKCTL , &lctl); -if (lctl &PCI_EXP_LNKCTL_ASPM_L1 ) { + ret =pcie_capability_read_word (il->pci_dev,PCI_EXP_LNKCTL , &lctl); +if (!ret && (lctl &PCI_EXP_LNKCTL_ASPM_L1 )) {/* L1-ASPM enabled; disable(!) L0S */ il_set_bit (il,CSR_GIO_REG ,CSR_GIO_REG_VAL_L0S_ENABLED );
Signature · 6 lines
2.18.2 _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
Re: [PATCH 4/14 v3] iwlegacy: Check the return value of pcie_capability_read_*()
Quoted 6 lines · Saheed O. Bolarinwa
From: Bolarinwa Olayemi Saheed <[email protected]>
On failure
pcie_capability_read_dword()sets it's last parameter, val to 0. However, with Patch 14/14, it is possible that val is set to ~0 on failure. This would introduce a bug because (x & x) == (~0 & x).
Quoted 7 lines
This bug can be avoided without changing the function's behaviour if the return value of
pcie_capability_read_dwordis checked to confirm success.Check the return value of
pcie_capability_read_dword()to ensure success.Suggested-by Bjorn Helgaas <[email protected]>Signed-off-by Bolarinwa Olayemi Saheed <[email protected]>
Patch applied to wireless-drivers-next.git, thanks.
9018fd7f2a73 iwlegacy: Check the return value of pcie_capability_read_*()
Signature · 3 lines
https://patchwork.kernel.org/patch/11660739/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [Linux-kernel-mentees] [PATCH 4/14 v3] iwlegacy: Check the return value of pcie_capability_read_*()
In reply to Saheed O. BolarinwaQuoted 6 lines · Saheed O. Bolarinwa
From: Bolarinwa Olayemi Saheed <[email protected]>
On failure
pcie_capability_read_dword()sets it's last parameter, val to 0. However, with Patch 14/14, it is possible that val is set to ~0 on failure. This would introduce a bug because (x & x) == (~0 & x).
Quoted 7 lines
This bug can be avoided without changing the function's behaviour if the return value of
pcie_capability_read_dwordis checked to confirm success.Check the return value of
pcie_capability_read_dword()to ensure success.Suggested-by Bjorn Helgaas <[email protected]>Signed-off-by Bolarinwa Olayemi Saheed <[email protected]>
Patch applied to wireless-drivers-next.git, thanks.
9018fd7f2a73 iwlegacy: Check the return value of pcie_capability_read_*()
Signature · 8 lines
https://patchwork.kernel.org/patch/11660739/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
PATCH v3 8/14 PCI/ACPI: Check the return value of pcie_capability_read_*()
In reply to Saheed O. BolarinwaFrom: Bolarinwa Olayemi Saheed <[email protected]>
On failure pcie_capability_read_dword() sets it's last parameter, val to 0.
However, with Patch 14/14, it is possible that val is set to ~0 on failure. This would introduce a bug because (x & x) == (~0 & x).
This bug can be avoided if the return value of pcie_capability_read_word is checked to confirm success.
Check the return value of pcie_capability_read_word() to ensure success.
--- drivers/pci/pci-acpi.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
drivers/pci/pci-acpi.c +6 -4
@@ -248,12 +248,13 @@ static bool pcie_root_rcb_set(struct pci_dev *dev) {struct pci_dev *rp =pcie_find_root_port (dev);u16 lnkctl; +int ret;if (!rp)return false; -pcie_capability_read_word (rp,PCI_EXP_LNKCTL , &lnkctl); -if (lnkctl &PCI_EXP_LNKCTL_RCB ) + ret =pcie_capability_read_word (rp,PCI_EXP_LNKCTL , &lnkctl); +if (!ret && (lnkctl &PCI_EXP_LNKCTL_RCB ))return true;return false;
@@ -792,12 +793,13 @@ bool pciehp_is_native(struct pci_dev *bridge) {const struct pci_host_bridge *host;u32 slot_cap; +int ret;if (!IS_ENABLED (CONFIG_HOTPLUG_PCI_PCIE ))return false; -pcie_capability_read_dword (bridge,PCI_EXP_SLTCAP , &slot_cap); -if (!(slot_cap &PCI_EXP_SLTCAP_HPC )) + ret =pcie_capability_read_dword (bridge,PCI_EXP_SLTCAP , &slot_cap); +if (ret || !(slot_cap &PCI_EXP_SLTCAP_HPC ))return false;if (pcie_ports_native)
Signature · 6 lines
2.18.2 _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
Re: [PATCH 8/14 v3] PCI/ACPI: Check the return value of pcie_capability_read_*()
Quoted 8 lines · Saheed O. Bolarinwa
From: Bolarinwa Olayemi Saheed <[email protected]>
On failure
pcie_capability_read_dword()sets it's last parameter, val to 0.
However, with Patch 14/14, it is possible that val is set to ~0 on failure. This would introduce a bug because (x & x) == (~0 & x).
Quoted 7 lines
This bug can be avoided if the return value of
pcie_capability_read_wordis checked to confirm success.Check the return value of
pcie_capability_read_word()to ensure success.Suggested-by Bjorn Helgaas <[email protected]>Signed-off-by Bolarinwa Olayemi Saheed <[email protected]>
Quoted 43 lines
--- drivers/pci/pci-acpi.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)drivers/pci/pci-acpi.c +6
-5@@ -248,12 +248,13 @@ static bool pcie_root_rcb_set(struct pci_dev *dev) {struct pci_dev *rp =pcie_find_root_port (dev);u16 lnkctl; +int ret;if (!rp)return false; -pcie_capability_read_word (rp,PCI_EXP_LNKCTL , &lnkctl); -if (lnkctl &PCI_EXP_LNKCTL_RCB ) + ret =pcie_capability_read_word (rp,PCI_EXP_LNKCTL , &lnkctl); +if (!ret && (lnkctl &PCI_EXP_LNKCTL_RCB ))return true;return false;@@ -792,12 +793,13 @@ bool pciehp_is_native(struct pci_dev *bridge) {const struct pci_host_bridge *host;u32 slot_cap; +int ret;if (!IS_ENABLED (CONFIG_HOTPLUG_PCI_PCIE ))return false; -pcie_capability_read_dword (bridge,PCI_EXP_SLTCAP , &slot_cap); -if (!(slot_cap &PCI_EXP_SLTCAP_HPC )) + ret =pcie_capability_read_dword (bridge,PCI_EXP_SLTCAP , &slot_cap); +if (ret || !(slot_cap &PCI_EXP_SLTCAP_HPC ))return false;if (pcie_ports_native) --2.18.2
PATCH v3 12/14 PCI/AER: Check the return value of pcie_capability_read_*()
In reply to Saheed O. BolarinwaFrom: Bolarinwa Olayemi Saheed <[email protected]>
On failure pcie_capability_read_dword() sets it's last parameter, val to 0.
However, with Patch 14/14, it is possible that val is set to ~0 on failure. This would introduce a bug because (x & x) == (~0 & x).
This bug can be avoided if the return value of pcie_capability_read_word is checked to confirm success.
Check the return value of pcie_capability_read_word() to ensure success.
--- drivers/pci/pcie/aer.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/pci/pcie/aer.c +3 -2
@@ -800,6 +800,7 @@ static bool is_error_source(struct pci_dev *dev, struct aer_err_info *e_info)int aer = dev->aer_cap;u32 status, mask;u16 reg16; +int ret;/* * When bus id is equal to 0, it might be a bad id
@@ -828,8 +829,8 @@ static bool is_error_source(struct pci_dev *dev, struct aer_err_info *e_info)return false;/* Check if AER is enabled */ -pcie_capability_read_word (dev,PCI_EXP_DEVCTL , ®16); -if (!(reg16 &PCI_EXP_AER_FLAGS )) + ret =pcie_capability_read_word (dev,PCI_EXP_DEVCTL , ®16); +if (ret || !(reg16 &PCI_EXP_AER_FLAGS ))return false;if (!aer)
Signature · 6 lines
2.18.2 _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
Quoted 38 lines · Saheed O. Bolarinwa
From: Bolarinwa Olayemi Saheed <[email protected]>
On failure
pcie_capability_read_dword()sets it's last parameter, val to 0. In this case dn and up will be 0, soaspm_hw_l1_supported()will return false.
However, with Patch 14/14, it is possible that val is set to ~0 on failure. This would introduce a bug because (x & x) == (~0 & x). So with dn and up being0x02, a true value is return when the read has actually failed.This bug can be avoided if the return value of
pcie_capability_read_dwordis checked to confirm success. The behaviour of the function remains intact.Check the return value of
pcie_capability_read_dword()to ensure success.Suggested-by Bjorn Helgaas <[email protected]>Signed-off-by Bolarinwa Olayemi Saheed <[email protected]>drivers/infiniband/hw/hfi1/aspm.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)drivers/infiniband/hw/hfi1/aspm.c +3
-1+++ b/drivers/infiniband/hw/hfi1/aspm.c@@ -24,6 +24,7 @@ static bool aspm_hw_l1_supported(struct hfi1_devdata *dd) {struct pci_dev *parent = dd->pcidev->bus->self;u32 up, dn; +int ret_up, ret_dn;/* * If the driver does not have access to the upstream component, @@ -32,14 +33,14 @@ static bool aspm_hw_l1_supported(struct hfi1_devdata *dd)if (!parent)return false; -pcie_capability_read_dword (dd->pcidev,PCI_EXP_LNKCAP , &dn); + ret_dn =pcie_capability_read_dword (dd->pcidev,PCI_EXP_LNKCAP , &dn);
drivers/infiniband/hw/hfi1/aspm.c static bool aspm_hw_l1_supported(struct hfi1_devdata *dd) · line 44
Quoted 10 lines
drivers/infiniband/hw/hfi1/aspm.c +2
-2dn =ASPM_L1_SUPPORTED (dn); -pcie_capability_read_dword (parent,PCI_EXP_LNKCAP , &up); + ret_up =pcie_capability_read_dword (parent,PCI_EXP_LNKCAP , &up); up =ASPM_L1_SUPPORTED (up);/* ASPM works on A-step but is reported as not supported */ -return (!!dn ||is_ax (dd)) && !!up; +return !!ret_dn && !!ret_up && (!!dn ||is_ax (dd)) && !!up; }
what is all the !! for? boolean contexts already coerce to boolean
Jason