Messages in this thread Patch in this message | | | Date | Wed, 18 May 2022 10:18:53 +0800 | From | Hangbin Liu <> | Subject | Re: [PATCHv2 net] bonding: fix missed rcu protection |
| |
On Tue, May 17, 2022 at 01:32:58PM -0400, Jonathan Toppins wrote: > Signed-off-by: Jonathan Toppins <jtoppins@redhat.com> > --- > RESEND, list still didn't receive my last version > > The diffstat is slightly larger but IMO a slightly more readable version. > When I was reading v2 I found myself jumping around.
Hi Jon,
Thanks for the commit. But..
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c > index 38e152548126..f9d27b63c454 100644 > --- a/drivers/net/bonding/bond_main.c > +++ b/drivers/net/bonding/bond_main.c > @@ -5591,23 +5591,32 @@ static int bond_ethtool_get_ts_info(struct net_device *bond_dev, > const struct ethtool_ops *ops; > struct net_device *real_dev; > struct phy_device *phydev; > + int ret = 0; > > + rcu_read_lock(); > real_dev = bond_option_active_slave_get_rcu(bond); > - if (real_dev) { > - ops = real_dev->ethtool_ops; > - phydev = real_dev->phydev; > - > - if (phy_has_tsinfo(phydev)) { > - return phy_ts_info(phydev, info); > - } else if (ops->get_ts_info) { > - return ops->get_ts_info(real_dev, info); > - } > - } > + if (real_dev) > + dev_hold(real_dev); > + rcu_read_unlock(); > + > + if (!real_dev) > + goto software; > > + ops = real_dev->ethtool_ops; > + phydev = real_dev->phydev; > + > + if (phy_has_tsinfo(phydev)) > + ret = phy_ts_info(phydev, info); > + else if (ops->get_ts_info) > + ret = ops->get_ts_info(real_dev, info); else { dev_put(real_dev); goto software; }
Here we need another check and goto software if !phy_has_tsinfo() and no ops->get_ts_info. With this change we also have 2 goto and dev_put().
> + > + dev_put(real_dev); > + return ret; > + > +software: > info->so_timestamping = SOF_TIMESTAMPING_RX_SOFTWARE | > SOF_TIMESTAMPING_SOFTWARE; > info->phc_index = -1; > - > return 0; > }
As Jakub remind, dev_hold() and dev_put() can take NULL now. So how about this new patch:
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 38e152548126..b5c5196e03ee 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -5591,16 +5591,23 @@ static int bond_ethtool_get_ts_info(struct net_device *bond_dev, const struct ethtool_ops *ops; struct net_device *real_dev; struct phy_device *phydev; + int ret = 0; + rcu_read_lock(); real_dev = bond_option_active_slave_get_rcu(bond); + dev_hold(real_dev); + rcu_read_unlock(); + if (real_dev) { ops = real_dev->ethtool_ops; phydev = real_dev->phydev; if (phy_has_tsinfo(phydev)) { - return phy_ts_info(phydev, info); + ret = phy_ts_info(phydev, info); + goto out; } else if (ops->get_ts_info) { - return ops->get_ts_info(real_dev, info); + ret = ops->get_ts_info(real_dev, info); + goto out; } } @@ -5608,7 +5615,9 @@ static int bond_ethtool_get_ts_info(struct net_device *bond_dev, SOF_TIMESTAMPING_SOFTWARE; info->phc_index = -1; - return 0; +out: + dev_put(real_dev); + return ret; }
Thanks Hangbin
| |