[{"id":3686720,"web_url":"http://patchwork.ozlabs.org/comment/3686720/","msgid":"<20260505212941.17138C2BCB4@smtp.kernel.org>","list_archive_url":null,"date":"2026-05-05T21:29:40","subject":"Re: [PATCH v15 1/7] PCI: Allow per function PCI slots to fix slot\n reset on s390","submitter":{"id":93311,"url":"http://patchwork.ozlabs.org/api/people/93311/","name":null,"email":"sashiko-bot@kernel.org"},"content":"Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:\n- [High] Global enablement of `per_func_slot` on S390 breaks standard PCIe hotplug matching and hardware association.\n- [High] Lockless read of `dev->slot` in `pci_dev_reset_slot_function()` leads to a Time-Of-Check to Time-Of-Use (TOCTOU) Use-After-Free.\n--\n\ncommit e4e35148b2442f92d3db17a20a087722c65c043d\nAuthor: Farhan Ali \nPCI: Allow per function PCI slots to fix slot reset on s390\n\nThis commit introduces a per-function PCI slot flag to fix slot resets for\nmultifunction devices on s390 systems. It updates the PCI slot number field\nto a 16-bit integer to accommodate special placeholder and all-devices values.\n\n> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c\n> index 8f7cfcc000901..d0c9f0166af5c 100644\n> --- a/drivers/pci/pci.c\n> +++ b/drivers/pci/pci.c\n[ ... ]\n> @@ -4865,8 +4865,9 @@ static int pci_reset_hotplug_slot(struct hotplug_slot *hotplug, bool probe)\n> \n> static int pci_dev_reset_slot_function(struct pci_dev *dev, bool probe)\n> {\n> -\tif (dev->multifunction || dev->subordinate || !dev->slot ||\n> -\t dev->dev_flags & PCI_DEV_FLAGS_NO_BUS_RESET)\n> +\tif (dev->subordinate || !dev->slot ||\n> +\t dev->dev_flags & PCI_DEV_FLAGS_NO_BUS_RESET ||\n> +\t (dev->multifunction && !dev->slot->per_func_slot))\n\nDoes this code introduce a time-of-check to time-of-use use-after-free\nregression?\n\nWhen dev->slot is accessed locklessly, it could be freed concurrently if a\nhotplug driver module unload invokes pci_destroy_slot() and\npci_slot_release(), which clears dev->slot to NULL under pci_bus_sem and\nfrees the slot.\n\nBecause this function does not hold pci_bus_sem or elevate the slot's\nreference count, adding another lockless dereference to\ndev->slot->per_func_slot right after the !dev->slot check could access\nfreed memory if the slot is destroyed in between.\n\n> diff --git a/drivers/pci/slot.c b/drivers/pci/slot.c\n> index 6d5cd37bfb1e7..894d6213ed306 100644\n> --- a/drivers/pci/slot.c\n> +++ b/drivers/pci/slot.c\n[ ... ]\n> @@ -72,6 +72,23 @@ static ssize_t cur_speed_read_file(struct pci_slot *slot, char *buf)\n> \treturn bus_speed_read(slot->bus->cur_bus_speed, buf);\n> }\n> \n> +static bool pci_dev_matches_slot(struct pci_dev *dev, struct pci_slot *slot)\n> +{\n> +\tif (slot->per_func_slot)\n> +\t\treturn dev->devfn == slot->number;\n> +\n> +\treturn slot->number == PCI_SLOT_ALL_DEVICES ||\n> +\t\tPCI_SLOT(dev->devfn) == slot->number;\n> +}\n> +\n> +static bool pci_slot_enabled_per_func(void)\n> +{\n> +\tif (IS_ENABLED(CONFIG_S390))\n> +\t\treturn true;\n> +\n> +\treturn false;\n> +}\n\nDoes this global enablement cause a regression for standard PCIe hotplug\nmatching on s390 systems?\n\nBy unconditionally returning true on s390, slot->per_func_slot is globally\nset to 1 for all hotplug slots created. When per_func_slot is true,\npci_dev_matches_slot() prioritizes an exact match of\ndev->devfn == slot->number.\n\nHowever, standard hotplug drivers like pciehp initialize slots using\nPCI_SLOT_ALL_DEVICES (now 0xfeff). Since a device's devfn is an 8-bit value,\nthis will never match, which might completely break pciehp topology on s390\nenvironments such as KVM guests.\n\nAdditionally, if generic drivers like shpchp register slots using just the\ndevice number (for example, 5), the strict match will compare devfn to 5\n(device 0, function 5) instead of the correct device 5, function 0\n(devfn 40). Could this lead to failed associations or cross-device resets?","headers":{"Return-Path":"\n ","X-Original-To":["incoming@patchwork.ozlabs.org","linux-pci@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256\n header.s=k20201202 header.b=HsAAv3sD;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=172.234.253.10; helo=sea.lore.kernel.org;\n envelope-from=linux-pci+bounces-53795-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n\tdkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org\n header.b=\"HsAAv3sD\"","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=10.30.226.201"],"Received":["from sea.lore.kernel.org (sea.lore.kernel.org [172.234.253.10])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519 server-signature ECDSA (secp384r1) server-digest SHA384)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4g9BbT6xJXz1yJq\n\tfor ; Wed, 06 May 2026 07:33:57 +1000 (AEST)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby sea.lore.kernel.org (Postfix) with ESMTP id 4DE5B3037437\n\tfor ; Tue, 5 May 2026 21:29:42 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id BDE313644D1;\n\tTue, 5 May 2026 21:29:41 +0000 (UTC)","from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org\n [10.30.226.201])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id 98CC234B697\n\tfor ; Tue, 5 May 2026 21:29:41 +0000 (UTC)","by smtp.kernel.org (Postfix) with ESMTPSA id 17138C2BCB4;\n\tTue, 5 May 2026 21:29:41 +0000 (UTC)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1778016581; cv=none;\n b=YqvB7zCDU7kCzKvY0DRxF7o5ZmoC4ma11jJmvVkUCi9HhC86m80KAtEe+CCpm5qjCzjYor20wjYeZTwPy/CEtZxfYvSPo42uXbJBOYs0oQikvsviLxVtEoIwuMTGkYClNGEseBgiMUCIm4HV9WBhJF9Luvtzh2wOPUby/g3JMJc=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1778016581; c=relaxed/simple;\n\tbh=K3gL/QqX9XIpzqa2FbgJjpHhstp5sZGQSwwPjxPX41k=;\n\th=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Date:\n\t Message-Id;\n b=ctkCcwAtSIXThLluGTlKlYMJk/aeL1HM9nYaoxqYDy139gleZrTLI5j500VzKP2oReZyeh15IPbLzowBDpxy2Arsp2CXK+PbZZYrEkjaxhmjDy8SLa0l0lbE4pB8ceQwW26v2xlOEpoCtKagO4xONQLGzkJOJhxNex3CRQhkQdM=","ARC-Authentication-Results":"i=1; smtp.subspace.kernel.org;\n dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org\n header.b=HsAAv3sD; arc=none smtp.client-ip=10.30.226.201","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org;\n\ts=k20201202; t=1778016581;\n\tbh=K3gL/QqX9XIpzqa2FbgJjpHhstp5sZGQSwwPjxPX41k=;\n\th=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date:From;\n\tb=HsAAv3sDybVWQ/4TYLa5BpveuzqRvJBQC5YnWteQ+THMT7clsa6ScRPo3dnh0DQJI\n\t IEMlLflOVr9e9PKVojCbCnudyKUCRkcwGL6YsGl1iPLjHIlbFR1B3X9CI4mRVbk7gj\n\t ql/YKmp4wKn2vsIsNNswgtrNpnVZY7drd4mQ6oGf0ikSwGfohHcwnckISTnvj8PRjd\n\t RhIcKTREL78efpCOX13WMP2rYOjXHE12AOlgY80m2qKm3wIuIuh3nG/SZAMnGuPSJO\n\t koDtOtN2v7gefeiGBHuKB6fgtodpOyuSKQWNWJlZGwtpEA9A6O13OKcFYqwWde70KV\n\t Oie5ARf1ajiPQ==","From":"sashiko-bot@kernel.org","Subject":"Re: [PATCH v15 1/7] PCI: Allow per function PCI slots to fix slot\n reset on s390","Reply-To":"sashiko@lists.linux.dev","To":"\"Farhan Ali\" ","Cc":"linux-pci@vger.kernel.org","In-Reply-To":"<20260505200510.2954-2-alifm@linux.ibm.com>","References":"<20260505200510.2954-2-alifm@linux.ibm.com>","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","Date":"Tue, 05 May 2026 21:29:40 +0000","Message-Id":"<20260505212941.17138C2BCB4@smtp.kernel.org>","Precedence":"bulk","X-Mailing-List":"linux-pci@vger.kernel.org","List-Id":"","List-Subscribe":"","List-Unsubscribe":""}}]