| Message ID | 20210308031102.26730-1-baijiaju1990@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | net: bonding: fix error return code of bond_neigh_init() | expand |
On 2021-03-08 5:11 AM, Jia-Ju Bai wrote: > When slave is NULL or slave_ops->ndo_neigh_setup is NULL, no error > return code of bond_neigh_init() is assigned. > To fix this bug, ret is assigned with -EINVAL in these cases. > > Fixes: 9e99bfefdbce ("bonding: fix bond_neigh_init()") > Reported-by: TOTE Robot <oslab@tsinghua.edu.cn> > Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com> > --- > drivers/net/bonding/bond_main.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c > index 74cbbb22470b..456315bef3a8 100644 > --- a/drivers/net/bonding/bond_main.c > +++ b/drivers/net/bonding/bond_main.c > @@ -3978,11 +3978,15 @@ static int bond_neigh_init(struct neighbour *n) > > rcu_read_lock(); > slave = bond_first_slave_rcu(bond); > - if (!slave) > + if (!slave) { > + ret = -EINVAL; > goto out; > + } > slave_ops = slave->dev->netdev_ops; > - if (!slave_ops->ndo_neigh_setup) > + if (!slave_ops->ndo_neigh_setup) { > + ret = -EINVAL; > goto out; > + } > > /* TODO: find another way [1] to implement this. > * Passing a zeroed structure is fragile, > Hi, This breaks basic functionally that always worked. A slave doesn't need to exists nor to implement ndo_neigh_setup. Now trying to add a neigh entry because of that fails. This commit needs to be reverted. Thanks, Roi
On 2021/3/10 17:24, Roi Dayan wrote: > > > On 2021-03-08 5:11 AM, Jia-Ju Bai wrote: >> When slave is NULL or slave_ops->ndo_neigh_setup is NULL, no error >> return code of bond_neigh_init() is assigned. >> To fix this bug, ret is assigned with -EINVAL in these cases. >> >> Fixes: 9e99bfefdbce ("bonding: fix bond_neigh_init()") >> Reported-by: TOTE Robot <oslab@tsinghua.edu.cn> >> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com> >> --- >> drivers/net/bonding/bond_main.c | 8 ++++++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/bonding/bond_main.c >> b/drivers/net/bonding/bond_main.c >> index 74cbbb22470b..456315bef3a8 100644 >> --- a/drivers/net/bonding/bond_main.c >> +++ b/drivers/net/bonding/bond_main.c >> @@ -3978,11 +3978,15 @@ static int bond_neigh_init(struct neighbour *n) >> rcu_read_lock(); >> slave = bond_first_slave_rcu(bond); >> - if (!slave) >> + if (!slave) { >> + ret = -EINVAL; >> goto out; >> + } >> slave_ops = slave->dev->netdev_ops; >> - if (!slave_ops->ndo_neigh_setup) >> + if (!slave_ops->ndo_neigh_setup) { >> + ret = -EINVAL; >> goto out; >> + } >> /* TODO: find another way [1] to implement this. >> * Passing a zeroed structure is fragile, >> > > > Hi, > > This breaks basic functionally that always worked. A slave doesn't need > to exists nor to implement ndo_neigh_setup. > Now trying to add a neigh entry because of that fails. > This commit needs to be reverted. > Okay, thanks for the explanation, and I am sorry for this false report... Best wishes, Jia-Ju Bai
On 3/10/21 10:24 AM, Roi Dayan wrote: > > > On 2021-03-08 5:11 AM, Jia-Ju Bai wrote: >> When slave is NULL or slave_ops->ndo_neigh_setup is NULL, no error >> return code of bond_neigh_init() is assigned. >> To fix this bug, ret is assigned with -EINVAL in these cases. >> >> Fixes: 9e99bfefdbce ("bonding: fix bond_neigh_init()") >> Reported-by: TOTE Robot <oslab@tsinghua.edu.cn> >> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com> >> --- >> drivers/net/bonding/bond_main.c | 8 ++++++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c >> index 74cbbb22470b..456315bef3a8 100644 >> --- a/drivers/net/bonding/bond_main.c >> +++ b/drivers/net/bonding/bond_main.c >> @@ -3978,11 +3978,15 @@ static int bond_neigh_init(struct neighbour *n) >> rcu_read_lock(); >> slave = bond_first_slave_rcu(bond); >> - if (!slave) >> + if (!slave) { >> + ret = -EINVAL; >> goto out; >> + } >> slave_ops = slave->dev->netdev_ops; >> - if (!slave_ops->ndo_neigh_setup) >> + if (!slave_ops->ndo_neigh_setup) { >> + ret = -EINVAL; >> goto out; >> + } >> /* TODO: find another way [1] to implement this. >> * Passing a zeroed structure is fragile, >> > > > Hi, > > This breaks basic functionally that always worked. A slave doesn't need > to exists nor to implement ndo_neigh_setup. > Now trying to add a neigh entry because of that fails. > This commit needs to be reverted. > > Thanks, > Roi Agreed, this commit made no sense, please revert.
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Wed, 10 Mar 2021 17:55:04 +0100 > > Agreed, this commit made no sense, please revert. Done.
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 74cbbb22470b..456315bef3a8 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -3978,11 +3978,15 @@ static int bond_neigh_init(struct neighbour *n) rcu_read_lock(); slave = bond_first_slave_rcu(bond); - if (!slave) + if (!slave) { + ret = -EINVAL; goto out; + } slave_ops = slave->dev->netdev_ops; - if (!slave_ops->ndo_neigh_setup) + if (!slave_ops->ndo_neigh_setup) { + ret = -EINVAL; goto out; + } /* TODO: find another way [1] to implement this. * Passing a zeroed structure is fragile,
When slave is NULL or slave_ops->ndo_neigh_setup is NULL, no error return code of bond_neigh_init() is assigned. To fix this bug, ret is assigned with -EINVAL in these cases. Fixes: 9e99bfefdbce ("bonding: fix bond_neigh_init()") Reported-by: TOTE Robot <oslab@tsinghua.edu.cn> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com> --- drivers/net/bonding/bond_main.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)