net/bonding: fix segfault using invalid port
Checks
Commit Message
Port validation should be prior to getting dev data
to avoid segmentation fault. This patch fixed the issue.
Fixes: 6d72657ce379 ("net/bonding: add other aggregator modes")
Fixes: 112891cd27e5 ("net/bonding: add dedicated HW queues for LACP control")
Cc: stable@dpdk.org
Signed-off-by: Jiang JunyuX <junyux.jiang@intel.com>
---
drivers/net/bonding/rte_eth_bond_8023ad.c | 32 ++++++++++++++++-------
1 file changed, 22 insertions(+), 10 deletions(-)
Comments
Hi, Junyu
> -----Original Message-----
> From: Jiang, JunyuX
> Sent: Friday, October 25, 2019 4:56 AM
> To: dev@dpdk.org
> Cc: Chas Williams <chas3@att.com>; Yang, Qiming <qiming.yang@intel.com>;
> Jiang, JunyuX <junyux.jiang@intel.com>; stable@dpdk.org
> Subject: [PATCH] net/bonding: fix segfault using invalid port
>
> Port validation should be prior to getting dev data to avoid segmentation
> fault. This patch fixed the issue.
>
> Fixes: 6d72657ce379 ("net/bonding: add other aggregator modes")
> Fixes: 112891cd27e5 ("net/bonding: add dedicated HW queues for LACP
> control")
> Cc: stable@dpdk.org
>
> Signed-off-by: Jiang JunyuX <junyux.jiang@intel.com>
> ---
> drivers/net/bonding/rte_eth_bond_8023ad.c | 32 ++++++++++++++++-------
> 1 file changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/bonding/rte_eth_bond_8023ad.c
> b/drivers/net/bonding/rte_eth_bond_8023ad.c
> index 7d8da2b31..682854e39 100644
> --- a/drivers/net/bonding/rte_eth_bond_8023ad.c
> +++ b/drivers/net/bonding/rte_eth_bond_8023ad.c
> @@ -1387,11 +1387,12 @@
> rte_eth_bond_8023ad_agg_selection_set(uint16_t port_id,
> struct bond_dev_private *internals;
> struct mode8023ad_private *mode4;
>
> + if (valid_bonded_port_id(port_id) != 0)
> + return -EINVAL;
> +
> bond_dev = &rte_eth_devices[port_id];
> internals = bond_dev->data->dev_private;
>
> - if (valid_bonded_port_id(port_id) != 0)
> - return -EINVAL;
> if (internals->mode != 4)
> return -EINVAL;
>
> @@ -1408,11 +1409,12 @@ int
> rte_eth_bond_8023ad_agg_selection_get(uint16_t port_id)
> struct bond_dev_private *internals;
> struct mode8023ad_private *mode4;
>
> + if (valid_bonded_port_id(port_id) != 0)
> + return -EINVAL;
> +
> bond_dev = &rte_eth_devices[port_id];
> internals = bond_dev->data->dev_private;
>
> - if (valid_bonded_port_id(port_id) != 0)
> - return -EINVAL;
> if (internals->mode != 4)
> return -EINVAL;
> mode4 = &internals->mode4;
> @@ -1665,9 +1667,14 @@ int
> rte_eth_bond_8023ad_dedicated_queues_enable(uint16_t port) {
> int retval = 0;
> - struct rte_eth_dev *dev = &rte_eth_devices[port];
> - struct bond_dev_private *internals = (struct bond_dev_private *)
> - dev->data->dev_private;
> + struct rte_eth_dev *dev;
> + struct bond_dev_private *internals;
> +
> + if (valid_bonded_port_id(port) != 0)
> + return -EINVAL;
> +
> + dev = &rte_eth_devices[port];
> + internals = dev->data->dev_private;
Have you build success? I think we need to add (struct bond_dev_private *) for force transfer
>
> if (check_for_bonded_ethdev(dev) != 0)
> return -1;
> @@ -1689,9 +1696,14 @@ int
> rte_eth_bond_8023ad_dedicated_queues_disable(uint16_t port) {
> int retval = 0;
> - struct rte_eth_dev *dev = &rte_eth_devices[port];
> - struct bond_dev_private *internals = (struct bond_dev_private *)
> - dev->data->dev_private;
> + struct rte_eth_dev *dev;
> + struct bond_dev_private *internals;
> +
> + if (valid_bonded_port_id(port) != 0)
> + return -EINVAL;
> +
> + dev = &rte_eth_devices[port];
> + internals = dev->data->dev_private;
Same as before
>
> if (check_for_bonded_ethdev(dev) != 0)
> return -1;
> --
> 2.17.1
On 10/28, Yang, Qiming wrote:
>Hi, Junyu
>
>> + dev = &rte_eth_devices[port];
>> + internals = dev->data->dev_private;
>Have you build success? I think we need to add (struct bond_dev_private *) for force transfer
dev_private is a void *, an explicit type conversion is not needed here.
Thanks,
Xiaolong
>
>>
>> if (check_for_bonded_ethdev(dev) != 0)
>> return -1;
>> @@ -1689,9 +1696,14 @@ int
>> rte_eth_bond_8023ad_dedicated_queues_disable(uint16_t port) {
>> int retval = 0;
>> - struct rte_eth_dev *dev = &rte_eth_devices[port];
>> - struct bond_dev_private *internals = (struct bond_dev_private *)
>> - dev->data->dev_private;
>> + struct rte_eth_dev *dev;
>> + struct bond_dev_private *internals;
>> +
>> + if (valid_bonded_port_id(port) != 0)
>> + return -EINVAL;
>> +
>> + dev = &rte_eth_devices[port];
>> + internals = dev->data->dev_private;
>Same as before
>
>>
>> if (check_for_bonded_ethdev(dev) != 0)
>> return -1;
>> --
>> 2.17.1
>
> -----Original Message-----
> From: Ye, Xiaolong
> Sent: Tuesday, October 29, 2019 11:04 AM
> To: Yang, Qiming <qiming.yang@intel.com>
> Cc: Jiang, JunyuX <junyux.jiang@intel.com>; dev@dpdk.org; Chas Williams
> <chas3@att.com>; stable@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] net/bonding: fix segfault using invalid port
>
> On 10/28, Yang, Qiming wrote:
> >Hi, Junyu
> >
> >> + dev = &rte_eth_devices[port];
> >> + internals = dev->data->dev_private;
> >Have you build success? I think we need to add (struct bond_dev_private *)
> for force transfer
>
> dev_private is a void *, an explicit type conversion is not needed here.
OK, I saw the original code used.
- struct bond_dev_private *internals = (struct bond_dev_private *)
- dev->data->dev_private;
>
> Thanks,
> Xiaolong
> >
> >>
> >> if (check_for_bonded_ethdev(dev) != 0)
> >> return -1;
> >> @@ -1689,9 +1696,14 @@ int
> >> rte_eth_bond_8023ad_dedicated_queues_disable(uint16_t port) {
> >> int retval = 0;
> >> - struct rte_eth_dev *dev = &rte_eth_devices[port];
> >> - struct bond_dev_private *internals = (struct bond_dev_private *)
> >> - dev->data->dev_private;
> >> + struct rte_eth_dev *dev;
> >> + struct bond_dev_private *internals;
> >> +
> >> + if (valid_bonded_port_id(port) != 0)
> >> + return -EINVAL;
> >> +
> >> + dev = &rte_eth_devices[port];
> >> + internals = dev->data->dev_private;
> >Same as before
> >
> >>
> >> if (check_for_bonded_ethdev(dev) != 0)
> >> return -1;
> >> --
> >> 2.17.1
> >
On 10/29, Yang, Qiming wrote:
>> -----Original Message-----
>> From: Ye, Xiaolong
>> Sent: Tuesday, October 29, 2019 11:04 AM
>> To: Yang, Qiming <qiming.yang@intel.com>
>> Cc: Jiang, JunyuX <junyux.jiang@intel.com>; dev@dpdk.org; Chas Williams
>> <chas3@att.com>; stable@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH] net/bonding: fix segfault using invalid port
>>
>> On 10/28, Yang, Qiming wrote:
>> >Hi, Junyu
>> >
>> >> + dev = &rte_eth_devices[port];
>> >> + internals = dev->data->dev_private;
>> >Have you build success? I think we need to add (struct bond_dev_private *)
>> for force transfer
>>
>> dev_private is a void *, an explicit type conversion is not needed here.
>
>OK, I saw the original code used.
>- struct bond_dev_private *internals = (struct bond_dev_private *)
>- dev->data->dev_private;
The original cast is redundant, there is a patchset from Stephen before to
cleanup these unnecessary casts.
http://patches.dpdk.org/cover/53858/
Thanks,
Xiaolong
>
>>
>> Thanks,
>> Xiaolong
>> >
>> >>
>> >> if (check_for_bonded_ethdev(dev) != 0)
>> >> return -1;
>> >> @@ -1689,9 +1696,14 @@ int
>> >> rte_eth_bond_8023ad_dedicated_queues_disable(uint16_t port) {
>> >> int retval = 0;
>> >> - struct rte_eth_dev *dev = &rte_eth_devices[port];
>> >> - struct bond_dev_private *internals = (struct bond_dev_private *)
>> >> - dev->data->dev_private;
>> >> + struct rte_eth_dev *dev;
>> >> + struct bond_dev_private *internals;
>> >> +
>> >> + if (valid_bonded_port_id(port) != 0)
>> >> + return -EINVAL;
>> >> +
>> >> + dev = &rte_eth_devices[port];
>> >> + internals = dev->data->dev_private;
>> >Same as before
>> >
>> >>
>> >> if (check_for_bonded_ethdev(dev) != 0)
>> >> return -1;
>> >> --
>> >> 2.17.1
>> >
Thanks for the information, got it.
Qiming
> -----Original Message-----
> From: Ye, Xiaolong
> Sent: Tuesday, October 29, 2019 2:26 PM
> To: Yang, Qiming <qiming.yang@intel.com>
> Cc: Jiang, JunyuX <junyux.jiang@intel.com>; dev@dpdk.org; Chas Williams
> <chas3@att.com>; stable@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] net/bonding: fix segfault using invalid port
>
> On 10/29, Yang, Qiming wrote:
> >> -----Original Message-----
> >> From: Ye, Xiaolong
> >> Sent: Tuesday, October 29, 2019 11:04 AM
> >> To: Yang, Qiming <qiming.yang@intel.com>
> >> Cc: Jiang, JunyuX <junyux.jiang@intel.com>; dev@dpdk.org; Chas
> >> Williams <chas3@att.com>; stable@dpdk.org
> >> Subject: Re: [dpdk-dev] [PATCH] net/bonding: fix segfault using
> >> invalid port
> >>
> >> On 10/28, Yang, Qiming wrote:
> >> >Hi, Junyu
> >> >
> >> >> + dev = &rte_eth_devices[port];
> >> >> + internals = dev->data->dev_private;
> >> >Have you build success? I think we need to add (struct
> >> >bond_dev_private *)
> >> for force transfer
> >>
> >> dev_private is a void *, an explicit type conversion is not needed here.
> >
> >OK, I saw the original code used.
> >- struct bond_dev_private *internals = (struct bond_dev_private *)
> >- dev->data->dev_private;
>
> The original cast is redundant, there is a patchset from Stephen before to
> cleanup these unnecessary casts.
>
> http://patches.dpdk.org/cover/53858/
>
> Thanks,
> Xiaolong
>
>
> >
> >>
> >> Thanks,
> >> Xiaolong
> >> >
> >> >>
> >> >> if (check_for_bonded_ethdev(dev) != 0)
> >> >> return -1;
> >> >> @@ -1689,9 +1696,14 @@ int
> >> >> rte_eth_bond_8023ad_dedicated_queues_disable(uint16_t port) {
> >> >> int retval = 0;
> >> >> - struct rte_eth_dev *dev = &rte_eth_devices[port];
> >> >> - struct bond_dev_private *internals = (struct bond_dev_private *)
> >> >> - dev->data->dev_private;
> >> >> + struct rte_eth_dev *dev;
> >> >> + struct bond_dev_private *internals;
> >> >> +
> >> >> + if (valid_bonded_port_id(port) != 0)
> >> >> + return -EINVAL;
> >> >> +
> >> >> + dev = &rte_eth_devices[port];
> >> >> + internals = dev->data->dev_private;
> >> >Same as before
> >> >
> >> >>
> >> >> if (check_for_bonded_ethdev(dev) != 0)
> >> >> return -1;
> >> >> --
> >> >> 2.17.1
> >> >
@@ -1387,11 +1387,12 @@ rte_eth_bond_8023ad_agg_selection_set(uint16_t port_id,
struct bond_dev_private *internals;
struct mode8023ad_private *mode4;
+ if (valid_bonded_port_id(port_id) != 0)
+ return -EINVAL;
+
bond_dev = &rte_eth_devices[port_id];
internals = bond_dev->data->dev_private;
- if (valid_bonded_port_id(port_id) != 0)
- return -EINVAL;
if (internals->mode != 4)
return -EINVAL;
@@ -1408,11 +1409,12 @@ int rte_eth_bond_8023ad_agg_selection_get(uint16_t port_id)
struct bond_dev_private *internals;
struct mode8023ad_private *mode4;
+ if (valid_bonded_port_id(port_id) != 0)
+ return -EINVAL;
+
bond_dev = &rte_eth_devices[port_id];
internals = bond_dev->data->dev_private;
- if (valid_bonded_port_id(port_id) != 0)
- return -EINVAL;
if (internals->mode != 4)
return -EINVAL;
mode4 = &internals->mode4;
@@ -1665,9 +1667,14 @@ int
rte_eth_bond_8023ad_dedicated_queues_enable(uint16_t port)
{
int retval = 0;
- struct rte_eth_dev *dev = &rte_eth_devices[port];
- struct bond_dev_private *internals = (struct bond_dev_private *)
- dev->data->dev_private;
+ struct rte_eth_dev *dev;
+ struct bond_dev_private *internals;
+
+ if (valid_bonded_port_id(port) != 0)
+ return -EINVAL;
+
+ dev = &rte_eth_devices[port];
+ internals = dev->data->dev_private;
if (check_for_bonded_ethdev(dev) != 0)
return -1;
@@ -1689,9 +1696,14 @@ int
rte_eth_bond_8023ad_dedicated_queues_disable(uint16_t port)
{
int retval = 0;
- struct rte_eth_dev *dev = &rte_eth_devices[port];
- struct bond_dev_private *internals = (struct bond_dev_private *)
- dev->data->dev_private;
+ struct rte_eth_dev *dev;
+ struct bond_dev_private *internals;
+
+ if (valid_bonded_port_id(port) != 0)
+ return -EINVAL;
+
+ dev = &rte_eth_devices[port];
+ internals = dev->data->dev_private;
if (check_for_bonded_ethdev(dev) != 0)
return -1;