From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 9F6DAA0524; Mon, 7 Dec 2020 15:07:23 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id F0221F12; Mon, 7 Dec 2020 15:07:21 +0100 (CET) Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) by dpdk.org (Postfix) with ESMTP id 54FE0A3; Mon, 7 Dec 2020 15:07:19 +0100 (CET) X-Originating-IP: 90.78.4.16 Received: from u256.net (lfbn-poi-1-1343-16.w90-78.abo.wanadoo.fr [90.78.4.16]) (Authenticated sender: grive@u256.net) by relay5-d.mail.gandi.net (Postfix) with ESMTPSA id 07D9B1C000F; Mon, 7 Dec 2020 14:07:16 +0000 (UTC) Date: Mon, 7 Dec 2020 15:07:11 +0100 From: =?utf-8?Q?Ga=C3=ABtan?= Rivet To: "Min Hu (Connor)" Cc: chas3@att.com, declan.doherty@intel.com, dev@dpdk.org, gowrishankar.m@linux.vnet.ibm.com, stable@dpdk.org Message-ID: <20201207140711.3mjklkmkjrfwts5g@u256.net> References: <1606357496-8650-1-git-send-email-humin29@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1606357496-8650-1-git-send-email-humin29@huawei.com> Subject: Re: [dpdk-dev] [v1, 2/2] bonding: fix PCI address comparison on non-pci ports X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 26/11/20 10:24 +0800, Min Hu (Connor) wrote: > what scenarios may cause bugs in old ways. > Could you give an example, thanks. Hello, For example in the following code: - RTE_ETH_FOREACH_DEV(i) { - pci_dev = RTE_ETH_DEV_TO_PCI(&rte_eth_devices[i]); - eth_pci_addr = &pci_dev->addr; All ethdev are iterated, before reading their supposed PCI addresses, and comparing those to the one passed in arguments. But not all ethdev will be PCI devices, so the cast is incorrect. It will do a containerof() on a structure that it supposes contains an rte_device at the offset 16 (two pointers accounting for the TAILQ_ENTRY()), but nothing prevents any other bus from implementing their device with any other layout. So the cast is wrong, but generally it will give out readable memory at least. Then the field ((eth_dev)->device)->addr will be read and compared against the input, with arbitrary data here. A scenario that could cause bug would be when another bus implements a device in such a way that it will write plausible binary rep of PCI addresses at the same offset, and match a request from a user. The bonding PMD would take the device over without properly checking that it is actually a PCI device. There have been APIs introduced in the EAL to simplify the iteration of devices, especially restricted to buses. Those APIs should be used instead. The current implementation is inefficient and wrong. It will work in most cases but can still trigger weird issues for users, especially in cases that bonding PMD devs won't generally encounter (with setups where multiple buses are used with a variety of devices). Regards, -- Gaëtan