Re: [PATCH rdma-nxt] i40iw: Do an RCU lookup in i40iw_add_ipv4_addr

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jan 17, 2020 at 03:47:20PM -0600, Shiraz Saleem wrote:
> From: "Shiraz Saleem" <shiraz.saleem@xxxxxxxxx>
> 
> The in_dev_for_each_ifa_rtnl iterator in i40iw_add_ipv4_addr
> requires that the rtnl lock be held. But the rtnl_trylock/unlock
> scheme in this function does not guarantee it.
> 
> Replace the rtnl locking with an RCU lookup since there are
> no netdev object updates in this function.
> 
> Fixes: 8e06af711bf2 ("i40iw: add main, hdr, status")
> Signed-off-by: Shiraz Saleem <shiraz.saleem@xxxxxxxxx>
> ---
>  drivers/infiniband/hw/i40iw/i40iw_main.c | 16 +++++-----------
>  1 file changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/i40iw/i40iw_main.c b/drivers/infiniband/hw/i40iw/i40iw_main.c
> index 2386143..d7a1219 100644
> --- a/drivers/infiniband/hw/i40iw/i40iw_main.c
> +++ b/drivers/infiniband/hw/i40iw/i40iw_main.c
> @@ -1212,22 +1212,19 @@ static void i40iw_add_ipv4_addr(struct i40iw_device *iwdev)
>  {
>  	struct net_device *dev;
>  	struct in_device *idev;
> -	bool got_lock = true;
>  	u32 ip_addr;
>  
> -	if (!rtnl_trylock())
> -		got_lock = false;
> -
> -	for_each_netdev(&init_net, dev) {
> +	rcu_read_lock();
> +	for_each_netdev_rcu(&init_net, dev) {
>  		if ((((rdma_vlan_dev_vlan_id(dev) < 0xFFFF) &&
>  		      (rdma_vlan_dev_real_dev(dev) == iwdev->netdev)) ||
>  		    (dev == iwdev->netdev)) && (dev->flags & IFF_UP)) {
>  			const struct in_ifaddr *ifa;

The rtnl does not just protect the pointers, but also values like
flags from changing. When using RCU any varient values of the dev
should be read with READ_ONCE, such as flags.

I'm not completely sure the vlan tests are OK under RCU either, but
netdev has historically been loose on its READ_ONCE/WRITE_ONCE
annotations..

Jason



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux