On Tue, Jan 09, 2018 at 03:58:58PM +0200, Leon Romanovsky wrote:
> From: Parav Pandit <parav@xxxxxxxxxxxx>
>
> Netdev ifindex can change while performing IPv6 rt6_lookup().
> Therefore access ifindex under rcu lock.
> This ensures that ifindex won't change while lookup is in progress.
>
> Fixes: f887f2ac87c2 ("IB/cma: Validate routing of incoming requests")
> Reviewed-by: Daniel Jurgens <danielj@xxxxxxxxxxxx>
> Signed-off-by: Parav Pandit <parav@xxxxxxxxxxxx>
> Signed-off-by: Leon Romanovsky <leon@xxxxxxxxxx>
> drivers/infiniband/core/cma.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> index 90dead30de4c..690ed4238d72 100644
> +++ b/drivers/infiniband/core/cma.c
> @@ -1333,11 +1333,15 @@ static bool validate_ipv6_net_dev(struct net_device *net_dev,
> #if IS_ENABLED(CONFIG_IPV6)
> const int strict = ipv6_addr_type(&dst_addr->sin6_addr) &
> IPV6_ADDR_LINKLOCAL;
> - struct rt6_info *rt = rt6_lookup(dev_net(net_dev), &dst_addr->sin6_addr,
> - &src_addr->sin6_addr, net_dev->ifindex,
> - strict);
> + struct rt6_info *rt;
> bool ret;
>
> + rcu_read_lock();
> + rt = rt6_lookup(dev_net(net_dev), &dst_addr->sin6_addr,
> + &src_addr->sin6_addr, net_dev->ifindex,
> + strict);
> + rcu_read_unlock();
This doesn't make sense to me.
rcu is not supposed to be used in cases where two variables can change
concurrently.
For instance if the ifindex is changed due to dev_change_net_namespace()
then you have this:
dev_net_set(dev, net);
if (__dev_get_by_index(net, dev->ifindex))
dev->ifindex = dev_new_index(net);
Racing with this:
rt = rt6_lookup(dev_net(net_dev), &dst_addr->sin6_addr,
&src_addr->sin6_addr, net_dev->ifindex,
strict);
And we will get a racy incoherent result for dev_net(net_dev) and
net_dev->ifindex.
Same comment for the next patch too.
It kind of looks to me like the locking scheme in the netdev side
shuts down the netdev while moving it. So the RCU protected test
should be if the netdev is DOWN ?
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
