Re: [dpdk-dev] [v1, 2/2] bonding: fix PCI address comparison on non-pci ports

["Gaëtan Rivet" <grive@u256.net>]

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