netdev - Re: [PATCH 1/1] ixgbe: replace rtnl_lock with rcu_read

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Sun, 5 Jun 2016 11:40:45 -0700
From:	Alexander Duyck <alexander.duyck@...il.com>
To:	zhuyj <zyjzyj2000@...il.com>
Cc:	"e1000-devel@...ts.sourceforge.net" 
	<e1000-devel@...ts.sourceforge.net>,
	Netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH 1/1] ixgbe: replace rtnl_lock with rcu_read_lock

On Sun, Jun 5, 2016 at 2:14 AM,  <zyjzyj2000@...il.com> wrote:
> From: Zhu Yanjun <zyjzyj2000@...il.com>
>
>
> Signed-off-by: Zhu Yanjun <zyjzyj2000@...il.com>
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index 088c47c..cb19cbc 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -6840,7 +6840,7 @@ static void ixgbe_watchdog_link_is_up(struct ixgbe_adapter *adapter)
>         netif_tx_wake_all_queues(adapter->netdev);
>
>         /* enable any upper devices */
> -       rtnl_lock();
> +       rcu_read_lock();
>         netdev_for_each_all_upper_dev_rcu(adapter->netdev, upper, iter) {
>                 if (netif_is_macvlan(upper)) {
>                         struct macvlan_dev *vlan = netdev_priv(upper);
> @@ -6849,7 +6849,7 @@ static void ixgbe_watchdog_link_is_up(struct ixgbe_adapter *adapter)
>                                 netif_tx_wake_all_queues(upper);
>                 }
>         }
> -       rtnl_unlock();
> +       rcu_read_unlock();
>
>         /* update the default user priority for VFs */
>         ixgbe_update_default_up(adapter);

The rtnl_lock is being used to prevent any changes to the upper
devices while the interface is going through and updating the Tx queue
configuration on them.  Without that lock you introduce possible bugs
since you could have queues freed or added while this loop is in the
middle of trying to update the state of it.

As a general rule you use rcu_read_lock when you are only reading an
RCU protected structure, you use rtnl_lock when you have to protect
the system from any other changes while you are updating network
configuration.  In this case netif_tx_wake_all_queues changes the
state of the upper device.  The use of rtnl_lock here is intentional
and it is best to just leave it as is unless you have some actual bug
you are seeing.

- Alex