linux-rdma.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] RDMA/siw: Fix IPv6 addr_list locking
@ 2019-08-27 16:49 Bernard Metzler
  2019-08-27 17:00 ` Jason Gunthorpe
  2019-08-27 22:01 ` Bernard Metzler
  0 siblings, 2 replies; 3+ messages in thread
From: Bernard Metzler @ 2019-08-27 16:49 UTC (permalink / raw)
  To: linux-rdma; +Cc: bvanassche, jgg, dledford, Bernard Metzler

Walking the address list of an inet6_dev requires
appropriate locking. Since the called function
siw_listen_address() may sleep, we have to use
rtnl_lock() instead of read_lock_bh().

Also introduces:
- sanity checks if we got a device from
  in_dev_get() or in6_dev_get().
- skipping IPv6 addresses flagged IFA_F_TENTATIVE
  or IFA_F_DEPRECATED

Reported-by: Bart Van Assche <bvanassche@acm.org>
Fixes: 6c52fdc244b5 ("rdma/siw: connection management")
Signed-off-by: Bernard Metzler <bmt@zurich.ibm.com>
---
 drivers/infiniband/sw/siw/siw_cm.c | 33 +++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c
index 1db5ad3d9580..c145b4ff4556 100644
--- a/drivers/infiniband/sw/siw/siw_cm.c
+++ b/drivers/infiniband/sw/siw/siw_cm.c
@@ -1962,6 +1962,10 @@ int siw_create_listen(struct iw_cm_id *id, int backlog)
 		struct sockaddr_in s_laddr, *s_raddr;
 		const struct in_ifaddr *ifa;
 
+		if (!in_dev) {
+			rv = -ENODEV;
+			goto out;
+		}
 		memcpy(&s_laddr, &id->local_addr, sizeof(s_laddr));
 		s_raddr = (struct sockaddr_in *)&id->remote_addr;
 
@@ -1991,22 +1995,27 @@ int siw_create_listen(struct iw_cm_id *id, int backlog)
 		struct sockaddr_in6 *s_laddr = &to_sockaddr_in6(id->local_addr),
 			*s_raddr = &to_sockaddr_in6(id->remote_addr);
 
+		if (!in6_dev) {
+			rv = -ENODEV;
+			goto out;
+		}
 		siw_dbg(id->device,
 			"laddr %pI6:%d, raddr %pI6:%d\n",
 			&s_laddr->sin6_addr, ntohs(s_laddr->sin6_port),
 			&s_raddr->sin6_addr, ntohs(s_raddr->sin6_port));
 
-		read_lock_bh(&in6_dev->lock);
-		list_for_each_entry(ifp, &in6_dev->addr_list, if_list) {
-			struct sockaddr_in6 bind_addr;
-
+		rtnl_lock();
+		list_for_each_entry_rcu(ifp, &in6_dev->addr_list, if_list) {
+			if (ifp->flags & (IFA_F_TENTATIVE | IFA_F_DEPRECATED))
+				continue;
 			if (ipv6_addr_any(&s_laddr->sin6_addr) ||
 			    ipv6_addr_equal(&s_laddr->sin6_addr, &ifp->addr)) {
-				bind_addr.sin6_family = AF_INET6;
-				bind_addr.sin6_port = s_laddr->sin6_port;
-				bind_addr.sin6_flowinfo = 0;
-				bind_addr.sin6_addr = ifp->addr;
-				bind_addr.sin6_scope_id = dev->ifindex;
+				struct sockaddr_in6 bind_addr  = {
+					.sin6_family = AF_INET6,
+					.sin6_port = s_laddr->sin6_port,
+					.sin6_flowinfo = 0,
+					.sin6_addr = ifp->addr,
+					.sin6_scope_id = dev->ifindex };
 
 				rv = siw_listen_address(id, backlog,
 						(struct sockaddr *)&bind_addr,
@@ -2015,12 +2024,12 @@ int siw_create_listen(struct iw_cm_id *id, int backlog)
 					listeners++;
 			}
 		}
-		read_unlock_bh(&in6_dev->lock);
-
+		rtnl_unlock();
 		in6_dev_put(in6_dev);
 	} else {
-		return -EAFNOSUPPORT;
+		rv = -EAFNOSUPPORT;
 	}
+out:
 	if (listeners)
 		rv = 0;
 	else if (!rv)
-- 
2.17.2


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] RDMA/siw: Fix IPv6 addr_list locking
  2019-08-27 16:49 [PATCH v2] RDMA/siw: Fix IPv6 addr_list locking Bernard Metzler
@ 2019-08-27 17:00 ` Jason Gunthorpe
  2019-08-27 22:01 ` Bernard Metzler
  1 sibling, 0 replies; 3+ messages in thread
From: Jason Gunthorpe @ 2019-08-27 17:00 UTC (permalink / raw)
  To: Bernard Metzler; +Cc: linux-rdma, bvanassche, dledford

On Tue, Aug 27, 2019 at 06:49:55PM +0200, Bernard Metzler wrote:
> Walking the address list of an inet6_dev requires
> appropriate locking. Since the called function
> siw_listen_address() may sleep, we have to use
> rtnl_lock() instead of read_lock_bh().
> 
> Also introduces:
> - sanity checks if we got a device from
>   in_dev_get() or in6_dev_get().
> - skipping IPv6 addresses flagged IFA_F_TENTATIVE
>   or IFA_F_DEPRECATED
> 
> Reported-by: Bart Van Assche <bvanassche@acm.org>
> Fixes: 6c52fdc244b5 ("rdma/siw: connection management")
> Signed-off-by: Bernard Metzler <bmt@zurich.ibm.com>
>  drivers/infiniband/sw/siw/siw_cm.c | 33 +++++++++++++++++++-----------
>  1 file changed, 21 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c
> index 1db5ad3d9580..c145b4ff4556 100644
> +++ b/drivers/infiniband/sw/siw/siw_cm.c
> @@ -1962,6 +1962,10 @@ int siw_create_listen(struct iw_cm_id *id, int backlog)
>  		struct sockaddr_in s_laddr, *s_raddr;
>  		const struct in_ifaddr *ifa;
>  
> +		if (!in_dev) {
> +			rv = -ENODEV;
> +			goto out;
> +		}
>  		memcpy(&s_laddr, &id->local_addr, sizeof(s_laddr));
>  		s_raddr = (struct sockaddr_in *)&id->remote_addr;
>  
> @@ -1991,22 +1995,27 @@ int siw_create_listen(struct iw_cm_id *id, int backlog)
>  		struct sockaddr_in6 *s_laddr = &to_sockaddr_in6(id->local_addr),
>  			*s_raddr = &to_sockaddr_in6(id->remote_addr);
>  
> +		if (!in6_dev) {
> +			rv = -ENODEV;
> +			goto out;
> +		}
>  		siw_dbg(id->device,
>  			"laddr %pI6:%d, raddr %pI6:%d\n",
>  			&s_laddr->sin6_addr, ntohs(s_laddr->sin6_port),
>  			&s_raddr->sin6_addr, ntohs(s_raddr->sin6_port));
>  
> -		read_lock_bh(&in6_dev->lock);
> -		list_for_each_entry(ifp, &in6_dev->addr_list, if_list) {
> -			struct sockaddr_in6 bind_addr;
> -
> +		rtnl_lock();
> +		list_for_each_entry_rcu(ifp, &in6_dev->addr_list, if_list) {

If not holding RCU then don't use the rcu list iterator..

Jason

^ permalink raw reply	[flat|nested] 3+ messages in thread

* RE: [PATCH v2] RDMA/siw: Fix IPv6 addr_list locking
  2019-08-27 16:49 [PATCH v2] RDMA/siw: Fix IPv6 addr_list locking Bernard Metzler
  2019-08-27 17:00 ` Jason Gunthorpe
@ 2019-08-27 22:01 ` Bernard Metzler
  1 sibling, 0 replies; 3+ messages in thread
From: Bernard Metzler @ 2019-08-27 22:01 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: linux-rdma, bvanassche, dledford

-----"Jason Gunthorpe" <jgg@ziepe.ca> wrote: -----

>To: "Bernard Metzler" <bmt@zurich.ibm.com>
>From: "Jason Gunthorpe" <jgg@ziepe.ca>
>Date: 08/27/2019 07:28PM
>Cc: linux-rdma@vger.kernel.org, bvanassche@acm.org,
>dledford@redhat.com
>Subject: [EXTERNAL] Re: [PATCH v2] RDMA/siw: Fix IPv6 addr_list
>locking
>
>On Tue, Aug 27, 2019 at 06:49:55PM +0200, Bernard Metzler wrote:
>> Walking the address list of an inet6_dev requires
>> appropriate locking. Since the called function
>> siw_listen_address() may sleep, we have to use
>> rtnl_lock() instead of read_lock_bh().
>> 
>> Also introduces:
>> - sanity checks if we got a device from
>>   in_dev_get() or in6_dev_get().
>> - skipping IPv6 addresses flagged IFA_F_TENTATIVE
>>   or IFA_F_DEPRECATED
>> 
>> Reported-by: Bart Van Assche <bvanassche@acm.org>
>> Fixes: 6c52fdc244b5 ("rdma/siw: connection management")
>> Signed-off-by: Bernard Metzler <bmt@zurich.ibm.com>
>>  drivers/infiniband/sw/siw/siw_cm.c | 33
>+++++++++++++++++++-----------
>>  1 file changed, 21 insertions(+), 12 deletions(-)
>> 
>> diff --git a/drivers/infiniband/sw/siw/siw_cm.c
>b/drivers/infiniband/sw/siw/siw_cm.c
>> index 1db5ad3d9580..c145b4ff4556 100644
>> +++ b/drivers/infiniband/sw/siw/siw_cm.c
>> @@ -1962,6 +1962,10 @@ int siw_create_listen(struct iw_cm_id *id,
>int backlog)
>>  		struct sockaddr_in s_laddr, *s_raddr;
>>  		const struct in_ifaddr *ifa;
>>  
>> +		if (!in_dev) {
>> +			rv = -ENODEV;
>> +			goto out;
>> +		}
>>  		memcpy(&s_laddr, &id->local_addr, sizeof(s_laddr));
>>  		s_raddr = (struct sockaddr_in *)&id->remote_addr;
>>  
>> @@ -1991,22 +1995,27 @@ int siw_create_listen(struct iw_cm_id *id,
>int backlog)
>>  		struct sockaddr_in6 *s_laddr = &to_sockaddr_in6(id->local_addr),
>>  			*s_raddr = &to_sockaddr_in6(id->remote_addr);
>>  
>> +		if (!in6_dev) {
>> +			rv = -ENODEV;
>> +			goto out;
>> +		}
>>  		siw_dbg(id->device,
>>  			"laddr %pI6:%d, raddr %pI6:%d\n",
>>  			&s_laddr->sin6_addr, ntohs(s_laddr->sin6_port),
>>  			&s_raddr->sin6_addr, ntohs(s_raddr->sin6_port));
>>  
>> -		read_lock_bh(&in6_dev->lock);
>> -		list_for_each_entry(ifp, &in6_dev->addr_list, if_list) {
>> -			struct sockaddr_in6 bind_addr;
>> -
>> +		rtnl_lock();
>> +		list_for_each_entry_rcu(ifp, &in6_dev->addr_list, if_list) {
>
>If not holding RCU then don't use the rcu list iterator..
>

absolutely!


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-08-27 22:02 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-27 16:49 [PATCH v2] RDMA/siw: Fix IPv6 addr_list locking Bernard Metzler
2019-08-27 17:00 ` Jason Gunthorpe
2019-08-27 22:01 ` Bernard Metzler

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).