patches for DPDK stable branches
 help / color / mirror / Atom feed
From: Gaëtan Rivet <grive@u256.net>
To: "Min Hu (Connor)" <humin29@huawei.com>
Cc: chas3@att.com, declan.doherty@intel.com, dev@dpdk.org,
	gowrishankar.m@linux.vnet.ibm.com, stable@dpdk.org
Subject: Re: [dpdk-stable] [v1, 2/2] bonding: fix PCI address comparison on non-pci ports
Date: Mon, 7 Dec 2020 15:07:11 +0100
Message-ID: <20201207140711.3mjklkmkjrfwts5g@u256.net> (raw)
In-Reply-To: <1606357496-8650-1-git-send-email-humin29@huawei.com>

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

  reply	other threads:[~2020-12-07 14:07 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <cover.1587137703.git.grive@u256.net>
2020-04-17 16:42 ` [dpdk-stable] [PATCH v1 1/2] bonding: fix port id validity check on parsing Gaetan Rivet
2020-11-26  2:45   ` [dpdk-stable] [dpdk-dev][PATCH v1, " Min Hu (Connor)
2020-12-07 13:34     ` [dpdk-stable] [dpdk-dev] [PATCH " Ferruh Yigit
2020-04-17 16:42 ` [dpdk-stable] [PATCH v1 2/2] bonding: fix PCI address comparison on non-pci ports Gaetan Rivet
2020-11-26  2:24   ` [dpdk-stable] [v1, " Min Hu (Connor)
2020-12-07 14:07     ` Gaëtan Rivet [this message]
2020-12-16 12:14       ` Min Hu (Connor)
2020-12-17 10:53         ` Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201207140711.3mjklkmkjrfwts5g@u256.net \
    --to=grive@u256.net \
    --cc=chas3@att.com \
    --cc=declan.doherty@intel.com \
    --cc=dev@dpdk.org \
    --cc=gowrishankar.m@linux.vnet.ibm.com \
    --cc=humin29@huawei.com \
    --cc=stable@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

patches for DPDK stable branches

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/stable/0 stable/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 stable stable/ https://inbox.dpdk.org/stable \
		stable@dpdk.org
	public-inbox-index stable

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.stable


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git