lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20200801112446.149549-7-refactormyself@gmail.com>
Date:   Sat,  1 Aug 2020 13:24:35 +0200
From:   "Saheed O. Bolarinwa" <refactormyself@...il.com>
To:     helgaas@...nel.org, Borislav Petkov <bp@...en8.de>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Tony Luck <tony.luck@...el.com>
Cc:     "Saheed O. Bolarinwa" <refactormyself@...il.com>,
        bjorn@...gaas.com, skhan@...uxfoundation.org,
        linux-kernel-mentees@...ts.linuxfoundation.org,
        linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
        James Morse <james.morse@....com>,
        Robert Richter <rrichter@...vell.com>,
        linux-edac@...r.kernel.org
Subject: [RFC PATCH 06/17] edac: Drop uses of pci_read_config_*() return value

The return value of pci_read_config_*() may not indicate a device error.
However, the value read by these functions is more likely to indicate
this kind of error. This presents two overlapping ways of reporting
errors and complicates error checking.

It is possible to move to one single way of checking for error if the
dependency on the return value of these functions is removed, then it
can later be made to return void.

Remove all uses of the return value of pci_read_config_*().
Check the actual value read for ~0. In this case, ~0 is an invalid value
thus it indicates some kind of error.

drivers/edac/amd8111_edac.c edac_pci_read_dword() :
None of the callers of edac_pci_read_dword() uses the return value. The
obtained value can be checked for validity to confirm success.

Change the return type of edac_pci_read_dword() to void.

Suggested-by: Bjorn Helgaas <bjorn@...gaas.com>
Signed-off-by: Saheed O. Bolarinwa <refactormyself@...il.com>
---
 drivers/edac/amd64_edac.c      |  8 +++-----
 drivers/edac/amd8111_edac.c    | 16 +++++-----------
 drivers/edac/amd8131_edac.c    |  6 ++----
 drivers/edac/i82443bxgx_edac.c |  3 ++-
 drivers/edac/sb_edac.c         | 12 ++++++++----
 drivers/edac/skx_common.c      | 18 ++++++++++++------
 6 files changed, 32 insertions(+), 31 deletions(-)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 6262f6370c5d..f798eb17cb23 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -57,14 +57,12 @@ static const struct scrubrate {
 int __amd64_read_pci_cfg_dword(struct pci_dev *pdev, int offset,
 			       u32 *val, const char *func)
 {
-	int err = 0;
-
-	err = pci_read_config_dword(pdev, offset, val);
-	if (err)
+	pci_read_config_dword(pdev, offset, val);
+	if (*val == (u32)~0)
 		amd64_warn("%s: error reading F%dx%03x.\n",
 			   func, PCI_FUNC(pdev->devfn), offset);
 
-	return err;
+	return -ENODEV;
 }
 
 int __amd64_write_pci_cfg_dword(struct pci_dev *pdev, int offset,
diff --git a/drivers/edac/amd8111_edac.c b/drivers/edac/amd8111_edac.c
index 7508aa416ddb..ebf6deaf1d3d 100644
--- a/drivers/edac/amd8111_edac.c
+++ b/drivers/edac/amd8111_edac.c
@@ -34,24 +34,18 @@ enum amd8111_edac_pcis {
 };
 
 /* Wrapper functions for accessing PCI configuration space */
-static int edac_pci_read_dword(struct pci_dev *dev, int reg, u32 *val32)
+static void edac_pci_read_dword(struct pci_dev *dev, int reg, u32 *val32)
 {
-	int ret;
-
-	ret = pci_read_config_dword(dev, reg, val32);
-	if (ret != 0)
+	pci_read_config_dword(dev, reg, val32);
+	if (val32 == (u32)~0)
 		printk(KERN_ERR AMD8111_EDAC_MOD_STR
 			" PCI Access Read Error at 0x%x\n", reg);
-
-	return ret;
 }
 
 static void edac_pci_read_byte(struct pci_dev *dev, int reg, u8 *val8)
 {
-	int ret;
-
-	ret = pci_read_config_byte(dev, reg, val8);
-	if (ret != 0)
+	pci_read_config_byte(dev, reg, val8);
+	if (val8 == (u8)~0)
 		printk(KERN_ERR AMD8111_EDAC_MOD_STR
 			" PCI Access Read Error at 0x%x\n", reg);
 }
diff --git a/drivers/edac/amd8131_edac.c b/drivers/edac/amd8131_edac.c
index 169353710982..6df98c05391d 100644
--- a/drivers/edac/amd8131_edac.c
+++ b/drivers/edac/amd8131_edac.c
@@ -26,10 +26,8 @@
 /* Wrapper functions for accessing PCI configuration space */
 static void edac_pci_read_dword(struct pci_dev *dev, int reg, u32 *val32)
 {
-	int ret;
-
-	ret = pci_read_config_dword(dev, reg, val32);
-	if (ret != 0)
+	pci_read_config_dword(dev, reg, val32);
+	if (val32 == (u32)~0)
 		printk(KERN_ERR AMD8131_EDAC_MOD_STR
 			" PCI Access Read Error at 0x%x\n", reg);
 }
diff --git a/drivers/edac/i82443bxgx_edac.c b/drivers/edac/i82443bxgx_edac.c
index a2ca929e2168..d6797ed7ac65 100644
--- a/drivers/edac/i82443bxgx_edac.c
+++ b/drivers/edac/i82443bxgx_edac.c
@@ -243,7 +243,8 @@ static int i82443bxgx_edacmc_probe1(struct pci_dev *pdev, int dev_idx)
 	/* Something is really hosed if PCI config space reads from
 	 * the MC aren't working.
 	 */
-	if (pci_read_config_dword(pdev, I82443BXGX_NBXCFG, &nbxcfg))
+	pci_read_config_dword(pdev, I82443BXGX_NBXCFG, &nbxcfg);
+	if (nbxcfg == (u32)~0)
 		return -EIO;
 
 	layers[0].type = EDAC_MC_LAYER_CHIP_SELECT;
diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c
index d414698ca324..e56a06d68a4e 100644
--- a/drivers/edac/sb_edac.c
+++ b/drivers/edac/sb_edac.c
@@ -1697,13 +1697,15 @@ static int get_dimm_config(struct mem_ctl_info *mci)
 
 		if (knl_get_dimm_capacity(pvt, knl_mc_sizes) != 0)
 			return -1;
-		if (pci_read_config_dword(pvt->pci_ta, KNL_MCMTR, &pvt->info.mcmtr)) {
+		pci_read_config_dword(pvt->pci_ta, KNL_MCMTR, &pvt->info.mcmtr);
+		if (pvt->info.mcmtr == (u32)~0) {
 			edac_dbg(0, "Failed to read KNL_MCMTR register\n");
 			return -ENODEV;
 		}
 	} else {
 		if (pvt->info.type == HASWELL || pvt->info.type == BROADWELL) {
-			if (pci_read_config_dword(pvt->pci_ha, HASWELL_HASYSDEFEATURE2, &reg)) {
+			pci_read_config_dword(pvt->pci_ha, HASWELL_HASYSDEFEATURE2, &reg);
+			if (reg == (u32)~0) {
 				edac_dbg(0, "Failed to read HASWELL_HASYSDEFEATURE2 register\n");
 				return -ENODEV;
 			}
@@ -1714,7 +1716,8 @@ static int get_dimm_config(struct mem_ctl_info *mci)
 				goto next;
 			}
 		}
-		if (pci_read_config_dword(pvt->pci_ras, RASENABLES, &reg)) {
+		pci_read_config_dword(pvt->pci_ras, RASENABLES, &reg);
+		if (reg == (u32)~0) {
 			edac_dbg(0, "Failed to read RASENABLES register\n");
 			return -ENODEV;
 		}
@@ -1727,7 +1730,8 @@ static int get_dimm_config(struct mem_ctl_info *mci)
 		}
 
 next:
-		if (pci_read_config_dword(pvt->pci_ta, MCMTR, &pvt->info.mcmtr)) {
+		pci_read_config_dword(pvt->pci_ta, MCMTR, &pvt->info.mcmtr);
+		if (pvt->info.mcmtr == (u32)~0) {
 			edac_dbg(0, "Failed to read MCMTR register\n");
 			return -ENODEV;
 		}
diff --git a/drivers/edac/skx_common.c b/drivers/edac/skx_common.c
index 6d8d6dc626bf..7956c75c289f 100644
--- a/drivers/edac/skx_common.c
+++ b/drivers/edac/skx_common.c
@@ -161,7 +161,8 @@ int skx_get_src_id(struct skx_dev *d, int off, u8 *id)
 {
 	u32 reg;
 
-	if (pci_read_config_dword(d->util_all, off, &reg)) {
+	pci_read_config_dword(d->util_all, off, &reg);
+	if (reg == (u32)~0) {
 		skx_printk(KERN_ERR, "Failed to read src id\n");
 		return -ENODEV;
 	}
@@ -174,7 +175,8 @@ int skx_get_node_id(struct skx_dev *d, u8 *id)
 {
 	u32 reg;
 
-	if (pci_read_config_dword(d->util_all, 0xf4, &reg)) {
+	pci_read_config_dword(d->util_all, 0xf4, &reg);
+	if (reg == (u32)~0) {
 		skx_printk(KERN_ERR, "Failed to read node id\n");
 		return -ENODEV;
 	}
@@ -220,7 +222,8 @@ int skx_get_all_bus_mappings(struct res_config *cfg, struct list_head **list)
 			return -ENOMEM;
 		}
 
-		if (pci_read_config_dword(pdev, cfg->busno_cfg_offset, &reg)) {
+		pci_read_config_dword(pdev, cfg->busno_cfg_offset, &reg);
+		if (reg == (u32)~0) {
 			kfree(d);
 			pci_dev_put(pdev);
 			skx_printk(KERN_ERR, "Failed to read bus idx\n");
@@ -259,19 +262,22 @@ int skx_get_hi_lo(unsigned int did, int off[], u64 *tolm, u64 *tohm)
 		return -ENODEV;
 	}
 
-	if (pci_read_config_dword(pdev, off[0], &reg)) {
+	pci_read_config_dword(pdev, off[0], &reg);
+	if (reg == (u32)~0) {
 		skx_printk(KERN_ERR, "Failed to read tolm\n");
 		goto fail;
 	}
 	skx_tolm = reg;
 
-	if (pci_read_config_dword(pdev, off[1], &reg)) {
+	pci_read_config_dword(pdev, off[1], &reg);
+	if (reg == (u32)~0) {
 		skx_printk(KERN_ERR, "Failed to read lower tohm\n");
 		goto fail;
 	}
 	skx_tohm = reg;
 
-	if (pci_read_config_dword(pdev, off[2], &reg)) {
+	pci_read_config_dword(pdev, off[2], &reg);
+	if (reg == (u32)~0) {
 		skx_printk(KERN_ERR, "Failed to read upper tohm\n");
 		goto fail;
 	}
-- 
2.18.4

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ