All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hangbin Liu <liuhangbin@gmail.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: netdev@vger.kernel.org, Jay Vosburgh <j.vosburgh@gmail.com>,
	Veaceslav Falico <vfalico@gmail.com>,
	Andy Gospodarek <andy@greyhouse.net>,
	"David S . Miller" <davem@davemloft.net>,
	David Ahern <dsahern@gmail.com>,
	Jonathan Toppins <jtoppins@redhat.com>,
	Eric Dumazet <eric.dumazet@gmail.com>,
	Paolo Abeni <pabeni@redhat.com>,
	syzbot+92beb3d46aab498710fa@syzkaller.appspotmail.com,
	Vladimir Oltean <vladimir.oltean@nxp.com>
Subject: Re: [PATCH RESEND net] bonding: fix missed rcu protection
Date: Tue, 17 May 2022 11:42:54 +0800	[thread overview]
Message-ID: <YoMZvrPcgIm8k2b6@Laptop-X1> (raw)
In-Reply-To: <20220516181028.7dbbf918@kernel.org>

On Mon, May 16, 2022 at 06:10:28PM -0700, Jakub Kicinski wrote:
> Can't ->get_ts_info sleep now? It'd be a little sad to force it 
> to be atomic just because of one upper dev trying to be fancy.
> Maybe all we need to do is to take a ref on the real_dev?

Do you mean

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 38e152548126..b60450211579 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -5591,16 +5591,20 @@ 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;
 
 	real_dev = bond_option_active_slave_get_rcu(bond);
 	if (real_dev) {
+		dev_hold(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 +5612,10 @@ static int bond_ethtool_get_ts_info(struct net_device *bond_dev,
 				SOF_TIMESTAMPING_SOFTWARE;
 	info->phc_index = -1;
 
-	return 0;
+out:
+	if (real_dev)
+		dev_put(real_dev);
+	return ret;
 }


This look OK to me.

Vladimir, Jay, WDYT?

> 
> Also please add a Link: to the previous discussion, it'd have been
> useful to get the context in which Vladimir suggested this.

OK, I will.

Thanks
Hangbin

  reply	other threads:[~2022-05-17  3:43 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-13 10:33 [PATCH RESEND net] bonding: fix missed rcu protection Hangbin Liu
2022-05-13 13:23 ` Jonathan Toppins
2022-05-13 23:59 ` Vladimir Oltean
2022-05-17  1:10 ` Jakub Kicinski
2022-05-17  3:42   ` Hangbin Liu [this message]
2022-05-17  7:24     ` Paolo Abeni
2022-05-17  8:04       ` Hangbin Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YoMZvrPcgIm8k2b6@Laptop-X1 \
    --to=liuhangbin@gmail.com \
    --cc=andy@greyhouse.net \
    --cc=davem@davemloft.net \
    --cc=dsahern@gmail.com \
    --cc=eric.dumazet@gmail.com \
    --cc=j.vosburgh@gmail.com \
    --cc=jtoppins@redhat.com \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=syzbot+92beb3d46aab498710fa@syzkaller.appspotmail.com \
    --cc=vfalico@gmail.com \
    --cc=vladimir.oltean@nxp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.