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; 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; -- 2.17.1
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
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..cbab538e6 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 = (struct bond_dev_private *)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 = (struct bond_dev_private *)dev->data->dev_private; if (check_for_bonded_ethdev(dev) != 0) return -1; -- 2.17.1
Port validation should be prior to getting device data to avoid segment fault. This patch fixed the segment fault caused by invalid port using. 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..cbab538e6 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 = (struct bond_dev_private *)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 = (struct bond_dev_private *)dev->data->dev_private; if (check_for_bonded_ethdev(dev) != 0) return -1; -- 2.17.1
Port validation should be prior to getting device data to avoid segment fault. This patch fixed the segment fault caused by invalid port using. 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; 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; -- 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
> >> >
On 10/28/19 10:23 PM, Jiang JunyuX wrote: > Port validation should be prior to getting device data > to avoid segment fault. This patch fixed the segment fault > caused by invalid port using. > > 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> Acked-by: Chas Williams <chas3@att.com> Patchwork says that there is a build error but it doesn't seem to be related to this change. > --- > 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; > > 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; >
> -----Original Message-----
> From: Jiang, JunyuX
> Sent: Tuesday, October 29, 2019 10:24 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 v4] net/bonding: fix invalid port using
>
> Port validation should be prior to getting device data to avoid segment fault.
> This patch fixed the segment fault caused by invalid port using.
>
> 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;
>
> 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;
> --
> 2.17.1
Reviewed-by: Qiming Yang <qiming.yang@intel.com>
On 10/29/2019 3:59 PM, Chas Williams wrote:
> On 10/28/19 10:23 PM, Jiang JunyuX wrote:
>> Port validation should be prior to getting device data
>> to avoid segment fault. This patch fixed the segment fault
>> caused by invalid port using.
>>
>> 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>
>
> Acked-by: Chas Williams <chas3@att.com>
Applied to dpdk-next-net/master, thanks.