From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 51CEFA054D for ; Tue, 7 Jun 2022 19:38:16 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 417954161A; Tue, 7 Jun 2022 19:38:16 +0200 (CEST) Received: from wout1-smtp.messagingengine.com (wout1-smtp.messagingengine.com [64.147.123.24]) by mails.dpdk.org (Postfix) with ESMTP id BE12D4021D; Tue, 7 Jun 2022 19:38:14 +0200 (CEST) Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.west.internal (Postfix) with ESMTP id 5819D3200B26; Tue, 7 Jun 2022 13:38:13 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute5.internal (MEProxy); Tue, 07 Jun 2022 13:38:13 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= cc:cc:content-transfer-encoding:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to; s=fm2; t=1654623492; x= 1654709892; bh=ZdVFhfbjPrlXT1uUym5/+KlQYyNASZB1tckXNJk8jEk=; b=M cTEbKRn+b/JZ/i5DC0np4WRkKHtdKPh5hJZxaZAzpVpKNP09BMEtHGOixw83U7JT i/MH1K/DGuXUFV8ekHzeJvcs1wUdkAWvJcqV3LLASmNNdKgzbyptuV922vfHa7x7 7SlR3NJ/KM8TwfVUNR2+NdK+vrybGd/ZBZLoz7E+d1CLIvY119Tfja8a4rdW4bbe rhAYkG2vnmS7pRd0zeHaGsJTpga9zLSMlcQO7B6XEq+OxlGkzVdD0M8SABkbkELv wWArUZ2B1UvFKv5ZgBJkdTRT+6MWWp1D51zfDvj7v8BqTVEfYnSQLkgoPu6jzD7C ravRDDJVwtxb3fsfQOEIg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:date:date:feedback-id:feedback-id:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to:x-me-proxy:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm2; t=1654623492; x= 1654709892; bh=ZdVFhfbjPrlXT1uUym5/+KlQYyNASZB1tckXNJk8jEk=; b=u CSW/YMv5YDVRW9LpYFrSH5cYaWcTszRIDZ2C6oFDi3S8aEwyR75dsFN0NFkc157c JJGiBlXWwWh3FP2O/nm/nWKwIW09Ng7VL00P4H+NO5VJgFyep6EcRuCT1tblwLaQ ZwvyI57dW5iV81zloR9Wpl2RAfbFx+//6hb62dJuCnsyDaBkAJF+Msb1bm+kO5rL 0E9hVYbM9hNYnJtv/7z+FZWhwP9vw2rT1rvQVDnBMfUUsoYzXdgutRDl5zuFxyFN k3eonM7Eeizau7fIvdy91WamXuM/QPKOuEAoYF1TECOrWg0qOjRa1U99FVYnvIN2 8afnb/THnmiGMZEXCD0yw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvfedruddthedgudduhecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpefhvfevufffkfgjfhgggfgtsehtufertddttddvnecuhfhrohhmpefvhhho mhgrshcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqne cuggftrfgrthhtvghrnheptdejieeifeehtdffgfdvleetueeffeehueejgfeuteeftddt ieekgfekudehtdfgnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilh hfrhhomhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth X-ME-Proxy: Feedback-ID: i47234305:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 7 Jun 2022 13:38:11 -0400 (EDT) From: Thomas Monjalon To: Min Hu Cc: dev@dpdk.org, Huisong Li , stable@dpdk.org, Gaetan Rivet Subject: Re: [PATCH 1/2] bus/pci: fix a segfault when call callback Date: Tue, 07 Jun 2022 19:38:07 +0200 Message-ID: <27279391.ouqheUzb2q@thomas> In-Reply-To: <20220521070523.34983-2-humin29@huawei.com> References: <20220521070523.34983-1-humin29@huawei.com> <20220521070523.34983-2-humin29@huawei.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org 21/05/2022 09:05, Min Hu (Connor): > From: Huisong Li > > After the driver probe is executed, the callback in application will > be called. The callback in application may call some APIs which access the What is the "callback in application" ? Do you mean the callback on probing event like RTE_ETH_EVENT_NEW? > rte_pci_driver::driver by the device::driver pointer to get driver > information. If the rte_pci_device::device::driver pointer isn't pointed to > rte_pci_driver::driver in rte_pci_probe_one_driver, a segfault will occur. > For example, when ethdev driver probe completes, the callback in > application call rte_eth_dev_info_get which use dev->device->driver->name. > So rte_pci_device::device::driver should point to rte_pci_driver::driver > before executing the driver probe. I understand the need and I approve the move. > Fixes: c752998b5e2e ("pci: introduce library and driver") > Cc: stable@dpdk.org > > Signed-off-by: Huisong Li > Signed-off-by: Min Hu (Connor) > --- > drivers/bus/pci/pci_common.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c > index 4a3a87f24f..507a654779 100644 > --- a/drivers/bus/pci/pci_common.c > +++ b/drivers/bus/pci/pci_common.c > @@ -265,11 +265,22 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr, > dr->driver.name, dev->id.vendor_id, dev->id.device_id, > loc->domain, loc->bus, loc->devid, loc->function, > dev->device.numa_node); > + > + /* > + * After the driver probe is executed, the callback in application will > + * be called. The callback in application may call some APIs which use > + * dev->device.driver to get some driver information. If the driver > + * pointer isn't pointed to driver->driver here, a segfault will occur. > + */ I would like to make this comment simpler once I'm sure we share the same understanding. > + if (!already_probed) > + dev->device.driver = &dr->driver; > + > /* call the driver probe() function */ > ret = dr->probe(dr, dev); > if (already_probed) > return ret; /* no rollback if already succeeded earlier */ > if (ret) { > + dev->device.driver = NULL; > dev->driver = NULL; > if ((dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) && > /* Don't unmap if device is unsupported and > @@ -282,8 +293,6 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr, > dev->vfio_req_intr_handle = NULL; > rte_intr_instance_free(dev->intr_handle); > dev->intr_handle = NULL; > - } else { > - dev->device.driver = &dr->driver; > } > > return ret; >