From a69326e134d4ffa0d2d77bf1a4a62271c51c3051 Mon Sep 17 00:00:00 2001 From: Niklas Schnelle Date: Fri, 6 Aug 2021 12:11:16 +0200 Subject: s390/pci: fix use after free of zpci_dev [ Upstream commit 2a671f77ee49f3e78997b77fdee139467ff6a598 ] The struct pci_dev uses reference counting but zPCI assumed erroneously that the last reference would always be the local reference after calling pci_stop_and_remove_bus_device(). This is usually the case but not how reference counting works and thus inherently fragile. In fact one case where this causes a NULL pointer dereference when on an SRIOV device the function 0 was hot unplugged before another function of the same multi-function device. In this case the second function's pdev->sriov->dev reference keeps the struct pci_dev of function 0 alive even after the unplug. This bug was previously hidden by the fact that we were leaking the struct pci_dev which in turn means that it always outlived the struct zpci_dev. This was fixed in commit 0b13525c20fe ("s390/pci: fix leak of PCI device structure") exposing the broken behavior. Fix this by accounting for the long living reference a struct pci_dev has to its underlying struct zpci_dev via the zbus->function[] array and only release that in pcibios_release_device() ensuring that the struct pci_dev is not left with a dangling reference. This is a minimal fix in the future it would probably better to use fine grained reference counting for struct zpci_dev. Fixes: 05bc1be6db4b2 ("s390/pci: create zPCI bus") Cc: stable@vger.kernel.org Reviewed-by: Matthew Rosato Signed-off-by: Niklas Schnelle Signed-off-by: Vasily Gorbik Signed-off-by: Sasha Levin --- arch/s390/pci/pci.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'arch/s390/pci/pci.c') diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c index 1ae7a76ae97b..ca1a105e3b5d 100644 --- a/arch/s390/pci/pci.c +++ b/arch/s390/pci/pci.c @@ -558,9 +558,12 @@ static void zpci_cleanup_bus_resources(struct zpci_dev *zdev) int pcibios_add_device(struct pci_dev *pdev) { + struct zpci_dev *zdev = to_zpci(pdev); struct resource *res; int i; + /* The pdev has a reference to the zdev via its bus */ + zpci_zdev_get(zdev); if (pdev->is_physfn) pdev->no_vf_scan = 1; @@ -580,7 +583,10 @@ int pcibios_add_device(struct pci_dev *pdev) void pcibios_release_device(struct pci_dev *pdev) { + struct zpci_dev *zdev = to_zpci(pdev); + zpci_unmap_resources(pdev); + zpci_zdev_put(zdev); } int pcibios_enable_device(struct pci_dev *pdev, int mask) -- cgit v1.2.3 From 0404bf4a660ca8d5c82b8252f55f1f64506aa09a Mon Sep 17 00:00:00 2001 From: Niklas Schnelle Date: Wed, 21 Jul 2021 19:58:54 +0200 Subject: s390/pci: fix misleading rc in clp_set_pci_fn() [ Upstream commit f7addcdd527a6dddfebe20c358b87bdb95624612 ] Currently clp_set_pci_fn() always returns 0 as long as the CLP request itself succeeds even if the operation itself returns a response code other than CLP_RC_OK or CLP_RC_SETPCIFN_ALRDY. This is highly misleading because calling code assumes that a zero rc means that the operation was successful. Fix this by returning the response code or cc on failure with the exception of the special handling for CLP_RC_SETPCIFN_ALRDY. Also let's not assume that the returned function handle for CLP_RC_SETPCIFN_ALRDY is 0, we don't need it anyway. Reviewed-by: Matthew Rosato Signed-off-by: Niklas Schnelle Signed-off-by: Heiko Carstens Signed-off-by: Sasha Levin --- arch/s390/pci/pci.c | 7 ++++--- arch/s390/pci/pci_clp.c | 33 ++++++++++++++++----------------- 2 files changed, 20 insertions(+), 20 deletions(-) (limited to 'arch/s390/pci/pci.c') diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c index ca1a105e3b5d..0ddb1fe353dc 100644 --- a/arch/s390/pci/pci.c +++ b/arch/s390/pci/pci.c @@ -659,9 +659,10 @@ int zpci_enable_device(struct zpci_dev *zdev) { int rc; - rc = clp_enable_fh(zdev, ZPCI_NR_DMA_SPACES); - if (rc) + if (clp_enable_fh(zdev, ZPCI_NR_DMA_SPACES)) { + rc = -EIO; goto out; + } rc = zpci_dma_init_device(zdev); if (rc) @@ -684,7 +685,7 @@ int zpci_disable_device(struct zpci_dev *zdev) * The zPCI function may already be disabled by the platform, this is * detected in clp_disable_fh() which becomes a no-op. */ - return clp_disable_fh(zdev); + return clp_disable_fh(zdev) ? -EIO : 0; } EXPORT_SYMBOL_GPL(zpci_disable_device); diff --git a/arch/s390/pci/pci_clp.c b/arch/s390/pci/pci_clp.c index d3331596ddbe..0a0e8b8293be 100644 --- a/arch/s390/pci/pci_clp.c +++ b/arch/s390/pci/pci_clp.c @@ -213,15 +213,19 @@ out: } static int clp_refresh_fh(u32 fid); -/* - * Enable/Disable a given PCI function and update its function handle if - * necessary +/** + * clp_set_pci_fn() - Execute a command on a PCI function + * @zdev: Function that will be affected + * @nr_dma_as: DMA address space number + * @command: The command code to execute + * + * Returns: 0 on success, < 0 for Linux errors (e.g. -ENOMEM), and + * > 0 for non-success platform responses */ static int clp_set_pci_fn(struct zpci_dev *zdev, u8 nr_dma_as, u8 command) { struct clp_req_rsp_set_pci *rrb; int rc, retries = 100; - u32 fid = zdev->fid; rrb = clp_alloc_block(GFP_KERNEL); if (!rrb) @@ -245,17 +249,16 @@ static int clp_set_pci_fn(struct zpci_dev *zdev, u8 nr_dma_as, u8 command) } } while (rrb->response.hdr.rsp == CLP_RC_SETPCIFN_BUSY); - if (rc || rrb->response.hdr.rsp != CLP_RC_OK) { - zpci_err("Set PCI FN:\n"); - zpci_err_clp(rrb->response.hdr.rsp, rc); - } - if (!rc && rrb->response.hdr.rsp == CLP_RC_OK) { zdev->fh = rrb->response.fh; - } else if (!rc && rrb->response.hdr.rsp == CLP_RC_SETPCIFN_ALRDY && - rrb->response.fh == 0) { + } else if (!rc && rrb->response.hdr.rsp == CLP_RC_SETPCIFN_ALRDY) { /* Function is already in desired state - update handle */ - rc = clp_refresh_fh(fid); + rc = clp_refresh_fh(zdev->fid); + } else { + zpci_err("Set PCI FN:\n"); + zpci_err_clp(rrb->response.hdr.rsp, rc); + if (!rc) + rc = rrb->response.hdr.rsp; } clp_free_block(rrb); return rc; @@ -301,17 +304,13 @@ int clp_enable_fh(struct zpci_dev *zdev, u8 nr_dma_as) rc = clp_set_pci_fn(zdev, nr_dma_as, CLP_SET_ENABLE_PCI_FN); zpci_dbg(3, "ena fid:%x, fh:%x, rc:%d\n", zdev->fid, zdev->fh, rc); - if (rc) - goto out; - - if (zpci_use_mio(zdev)) { + if (!rc && zpci_use_mio(zdev)) { rc = clp_set_pci_fn(zdev, nr_dma_as, CLP_SET_ENABLE_MIO); zpci_dbg(3, "ena mio fid:%x, fh:%x, rc:%d\n", zdev->fid, zdev->fh, rc); if (rc) clp_disable_fh(zdev); } -out: return rc; } -- cgit v1.2.3 From 973c57c5e649481a7444ec79f29b213115cb8c2c Mon Sep 17 00:00:00 2001 From: Niklas Schnelle Date: Thu, 8 Jul 2021 14:55:42 +0200 Subject: s390: make PCI mio support a machine flag [ Upstream commit 3322ba0d7bea1e24ae464418626f6a15b69533ab ] Kernel support for the newer PCI mio instructions can be toggled off with the pci=nomio command line option which needs to integrate with common code PCI option parsing. However this option then toggles static branches which can't be toggled yet in an early_param() call. Thus commit 9964f396f1d0 ("s390: fix setting of mio addressing control") moved toggling the static branches to the PCI init routine. With this setup however we can't check for mio support outside the PCI code during early boot, i.e. before switching the static branches, which we need to be able to export this as an ELF HWCAP. Improve on this by turning mio availability into a machine flag that gets initially set based on CONFIG_PCI and the facility bit and gets toggled off if pci=nomio is found during PCI option parsing allowing simple access to this machine flag after early init. Reviewed-by: Heiko Carstens Signed-off-by: Niklas Schnelle Signed-off-by: Heiko Carstens Signed-off-by: Sasha Levin --- arch/s390/include/asm/setup.h | 2 ++ arch/s390/kernel/early.c | 4 ++++ arch/s390/pci/pci.c | 5 ++--- 3 files changed, 8 insertions(+), 3 deletions(-) (limited to 'arch/s390/pci/pci.c') diff --git a/arch/s390/include/asm/setup.h b/arch/s390/include/asm/setup.h index bdb242a1544e..75a2ecec2ab8 100644 --- a/arch/s390/include/asm/setup.h +++ b/arch/s390/include/asm/setup.h @@ -38,6 +38,7 @@ #define MACHINE_FLAG_NX BIT(15) #define MACHINE_FLAG_GS BIT(16) #define MACHINE_FLAG_SCC BIT(17) +#define MACHINE_FLAG_PCI_MIO BIT(18) #define LPP_MAGIC BIT(31) #define LPP_PID_MASK _AC(0xffffffff, UL) @@ -113,6 +114,7 @@ extern unsigned long mio_wb_bit_mask; #define MACHINE_HAS_NX (S390_lowcore.machine_flags & MACHINE_FLAG_NX) #define MACHINE_HAS_GS (S390_lowcore.machine_flags & MACHINE_FLAG_GS) #define MACHINE_HAS_SCC (S390_lowcore.machine_flags & MACHINE_FLAG_SCC) +#define MACHINE_HAS_PCI_MIO (S390_lowcore.machine_flags & MACHINE_FLAG_PCI_MIO) /* * Console mode. Override with conmode= diff --git a/arch/s390/kernel/early.c b/arch/s390/kernel/early.c index 705844f73934..985e1e755333 100644 --- a/arch/s390/kernel/early.c +++ b/arch/s390/kernel/early.c @@ -238,6 +238,10 @@ static __init void detect_machine_facilities(void) clock_comparator_max = -1ULL >> 1; __ctl_set_bit(0, 53); } + if (IS_ENABLED(CONFIG_PCI) && test_facility(153)) { + S390_lowcore.machine_flags |= MACHINE_FLAG_PCI_MIO; + /* the control bit is set during PCI initialization */ + } } static inline void save_vector_registers(void) diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c index 0ddb1fe353dc..f5ddbc625c1a 100644 --- a/arch/s390/pci/pci.c +++ b/arch/s390/pci/pci.c @@ -866,7 +866,6 @@ static void zpci_mem_exit(void) } static unsigned int s390_pci_probe __initdata = 1; -static unsigned int s390_pci_no_mio __initdata; unsigned int s390_pci_force_floating __initdata; static unsigned int s390_pci_initialized; @@ -877,7 +876,7 @@ char * __init pcibios_setup(char *str) return NULL; } if (!strcmp(str, "nomio")) { - s390_pci_no_mio = 1; + S390_lowcore.machine_flags &= ~MACHINE_FLAG_PCI_MIO; return NULL; } if (!strcmp(str, "force_floating")) { @@ -906,7 +905,7 @@ static int __init pci_base_init(void) if (!test_facility(69) || !test_facility(71)) return 0; - if (test_facility(153) && !s390_pci_no_mio) { + if (MACHINE_HAS_PCI_MIO) { static_branch_enable(&have_mio); ctl_set_bit(2, 5); } -- cgit v1.2.3