[DPDK/ethdev Bug 2049] net/af_packet and other vdevs segfaults when created with the global devargs syntax

[bugzilla@dpdk.org]

https://bugs.dpdk.org/show_bug.cgi?id=2049

            Bug ID: 2049
           Summary: net/af_packet and other vdevs segfaults when created
                    with the global devargs syntax
           Product: DPDK
           Version: 21.11
          Hardware: All
                OS: All
            Status: UNCONFIRMED
          Severity: minor
          Priority: Normal
         Component: ethdev
          Assignee: dev@dpdk.org
          Reporter: frank@dressler.pro
  Target Milestone: ---

DPDK can be crashed (SEGFAULT; `strdup(NULL)`) via unusual devargs.

```
dpdk-testpmd --vdev bus=vdev,name=net_vhost0
dpdk-testpmd --vdev bus=vdev,name=net_af_packet0
...
dpdk-testpmd --a bus=vdev,name=net_vhost0
dpdk-testpmd --a bus=vdev,name=net_af_packet0
...
```

Bug exists since 2021, commit b344eb5d941a, which added the "global device
syntax" (`bus=X,paramX=x/class=Y,paramY=y/driver=Z,paramZ=z`) that can be used
in `-a`/`-b`,`-vdev`. That change calls the other way "legacy" but is not a
fully-capable replacement since it lacks the ability to intentionally specify
`struct rte_devargs::args` that many PMDs rely on.

Cause for the crash is that `struct rte_devargs::args` is not set when the
"global device syntax" is used and some PMDs `strdup` it without checking for
`NULL`. The `args` sits in a union with `drv_str` in `struct rte_devargs`.
which can hence be set indirectly via, e.g.:

```
dpdk-testpmd --vdev bus=vdev,name=net_af_packet0/driver=abc
```

which would prevent the crash because that sets `drv_str` which is in a union
with the `args` that gets `strdup`ed. An almost working string would then look
as follows

```
dpdk-testpmd --vdev bus=vdev,name=net_af_packet0/driver=abc,iface=lo
```

since the AF_PACKET PMD requires the `iface`. But the AF_PACKET PMD would
reject that since `args` is the whole `driver=abc,iface=lo` and `driver` is not
among the acceptable keys.

I think the fix might be to add an "anonymous" (or "args") layer and
de`union`ize `args/`dev_str`. This would allow

```
dpdk-testpmd --vdev bus=vdev,name=net_af_packet0/driver=abc/iface=lo
```

Whatever layer does not start with `bus=`, `driver=`, or `class=` would be this
"anonymous"/"args" layer and end up in `struct rte_devargs::args` which could
be initialized to the empty string so that the existing code does not crash
when the `args` are not specified.

Alternatively, one could keep using the `driver` layer and either not pass down
the `driver=...` to the PMD or add `driver` to the list of allowed keys.

I have no idea how to properly fix it.

-- 
You are receiving this mail because:
You are the assignee for the bug.