ethdev: fix crash on owner delete
Checks
Commit Message
'eth_dev->data' can be null before ethdev allocated. The API walks
through all eth devices, at least for some data can be null.
Adding 'eth_dev->data' null check before accessing it.
Fixes: 33c73aae32e4 ("ethdev: allow ownership operations on unused port")
Cc: stable@dpdk.org
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
Cc: Matan Azrad <matan@nvidia.com>
---
lib/ethdev/rte_ethdev.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
Comments
> -----Original Message-----
> From: stable <stable-bounces@dpdk.org> On Behalf Of Ferruh Yigit
> Sent: Thursday, November 4, 2021 7:04 PM
> To: Thomas Monjalon <thomas@monjalon.net>; Andrew Rybchenko
> <andrew.rybchenko@oktetlabs.ru>; Stephen Hemminger
> <stephen@networkplumber.org>; Matan Azrad <matan@mellanox.com>
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org; stable@dpdk.org;
> Matan Azrad <matan@nvidia.com>
> Subject: [dpdk-stable] [PATCH] ethdev: fix crash on owner delete
>
> 'eth_dev->data' can be null before ethdev allocated. The API walks
> through all eth devices, at least for some data can be null.
>
> Adding 'eth_dev->data' null check before accessing it.
>
> Fixes: 33c73aae32e4 ("ethdev: allow ownership operations on unused port")
> Cc: stable@dpdk.org
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> Cc: Matan Azrad <matan@nvidia.com>
> ---
> lib/ethdev/rte_ethdev.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index 7db84b12d03b..8e679e4003db 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -757,10 +757,13 @@ rte_eth_dev_owner_delete(const uint64_t owner_id)
> rte_spinlock_lock(ð_dev_shared_data->ownership_lock);
>
> if (eth_is_valid_owner_id(owner_id)) {
> - for (port_id = 0; port_id < RTE_MAX_ETHPORTS; port_id++)
> - if (rte_eth_devices[port_id].data->owner.id == owner_id)
> - memset(&rte_eth_devices[port_id].data->owner, 0,
> + for (port_id = 0; port_id < RTE_MAX_ETHPORTS; port_id++) {
> + struct rte_eth_dev_data *data =
> + rte_eth_devices[port_id].data;
> + if (data != NULL && data->owner.id == owner_id)
> + memset(&data->owner, 0,
> sizeof(struct rte_eth_dev_owner));
> + }
> RTE_ETHDEV_LOG(NOTICE,
> "All port owners owned by %016"PRIx64" identifier have
> removed\n",
> owner_id);
> --
> 2.31.1
Acked-by: Chenbo Xia <chenbo.xia@intel.com>
05/11/2021 04:03, Xia, Chenbo:
> From: stable <stable-bounces@dpdk.org> On Behalf Of Ferruh Yigit
>
> > 'eth_dev->data' can be null before ethdev allocated. The API walks
> > through all eth devices, at least for some data can be null.
> >
> > Adding 'eth_dev->data' null check before accessing it.
> >
> > Fixes: 33c73aae32e4 ("ethdev: allow ownership operations on unused port")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
[...]
> > @@ -757,10 +757,13 @@ rte_eth_dev_owner_delete(const uint64_t owner_id)
> > rte_spinlock_lock(ð_dev_shared_data->ownership_lock);
> >
> > if (eth_is_valid_owner_id(owner_id)) {
> > - for (port_id = 0; port_id < RTE_MAX_ETHPORTS; port_id++)
> > - if (rte_eth_devices[port_id].data->owner.id == owner_id)
> > - memset(&rte_eth_devices[port_id].data->owner, 0,
> > + for (port_id = 0; port_id < RTE_MAX_ETHPORTS; port_id++) {
> > + struct rte_eth_dev_data *data =
> > + rte_eth_devices[port_id].data;
> > + if (data != NULL && data->owner.id == owner_id)
Indeed the NULL check was missing.
> Acked-by: Chenbo Xia <chenbo.xia@intel.com>
Acked-by: Thomas Monjalon <thomas@monjalon.net>
On 11/5/21 4:16 PM, Thomas Monjalon wrote:
> 05/11/2021 04:03, Xia, Chenbo:
>> From: stable <stable-bounces@dpdk.org> On Behalf Of Ferruh Yigit
>>
>>> 'eth_dev->data' can be null before ethdev allocated. The API walks
>>> through all eth devices, at least for some data can be null.
>>>
>>> Adding 'eth_dev->data' null check before accessing it.
>>>
>>> Fixes: 33c73aae32e4 ("ethdev: allow ownership operations on unused port")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> [...]
>>> @@ -757,10 +757,13 @@ rte_eth_dev_owner_delete(const uint64_t owner_id)
>>> rte_spinlock_lock(ð_dev_shared_data->ownership_lock);
>>>
>>> if (eth_is_valid_owner_id(owner_id)) {
>>> - for (port_id = 0; port_id < RTE_MAX_ETHPORTS; port_id++)
>>> - if (rte_eth_devices[port_id].data->owner.id == owner_id)
>>> - memset(&rte_eth_devices[port_id].data->owner, 0,
>>> + for (port_id = 0; port_id < RTE_MAX_ETHPORTS; port_id++) {
>>> + struct rte_eth_dev_data *data =
>>> + rte_eth_devices[port_id].data;
>>> + if (data != NULL && data->owner.id == owner_id)
>
> Indeed the NULL check was missing.
>
>> Acked-by: Chenbo Xia <chenbo.xia@intel.com>
>
> Acked-by: Thomas Monjalon <thomas@monjalon.net>
>
Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
On 11/5/2021 1:36 PM, Andrew Rybchenko wrote:
> On 11/5/21 4:16 PM, Thomas Monjalon wrote:
>> 05/11/2021 04:03, Xia, Chenbo:
>>> From: stable <stable-bounces@dpdk.org> On Behalf Of Ferruh Yigit
>>>
>>>> 'eth_dev->data' can be null before ethdev allocated. The API walks
>>>> through all eth devices, at least for some data can be null.
>>>>
>>>> Adding 'eth_dev->data' null check before accessing it.
>>>>
>>>> Fixes: 33c73aae32e4 ("ethdev: allow ownership operations on unused port")
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> [...]
>>>> @@ -757,10 +757,13 @@ rte_eth_dev_owner_delete(const uint64_t owner_id)
>>>> rte_spinlock_lock(ð_dev_shared_data->ownership_lock);
>>>>
>>>> if (eth_is_valid_owner_id(owner_id)) {
>>>> - for (port_id = 0; port_id < RTE_MAX_ETHPORTS; port_id++)
>>>> - if (rte_eth_devices[port_id].data->owner.id == owner_id)
>>>> - memset(&rte_eth_devices[port_id].data->owner, 0,
>>>> + for (port_id = 0; port_id < RTE_MAX_ETHPORTS; port_id++) {
>>>> + struct rte_eth_dev_data *data =
>>>> + rte_eth_devices[port_id].data;
>>>> + if (data != NULL && data->owner.id == owner_id)
>>
>> Indeed the NULL check was missing.
>>
>>> Acked-by: Chenbo Xia <chenbo.xia@intel.com>
>>
>> Acked-by: Thomas Monjalon <thomas@monjalon.net>
>>
>
> Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>
Applied to dpdk-next-net/main, thanks.
@@ -757,10 +757,13 @@ rte_eth_dev_owner_delete(const uint64_t owner_id)
rte_spinlock_lock(ð_dev_shared_data->ownership_lock);
if (eth_is_valid_owner_id(owner_id)) {
- for (port_id = 0; port_id < RTE_MAX_ETHPORTS; port_id++)
- if (rte_eth_devices[port_id].data->owner.id == owner_id)
- memset(&rte_eth_devices[port_id].data->owner, 0,
+ for (port_id = 0; port_id < RTE_MAX_ETHPORTS; port_id++) {
+ struct rte_eth_dev_data *data =
+ rte_eth_devices[port_id].data;
+ if (data != NULL && data->owner.id == owner_id)
+ memset(&data->owner, 0,
sizeof(struct rte_eth_dev_owner));
+ }
RTE_ETHDEV_LOG(NOTICE,
"All port owners owned by %016"PRIx64" identifier have removed\n",
owner_id);