diff mbox

[v2,01/11] RDMA/CMA: Mark IPv4 addresses correctly when the listener is IPv6

Message ID 1429520622-10303-2-git-send-email-haggaie@mellanox.com (mailing list archive)
State Rejected
Headers show

Commit Message

Haggai Eran April 20, 2015, 9:03 a.m. UTC 
From: Yotam Kenneth <yotamke@mellanox.com>

When accepting a new connection with the listener being IPv6, the
family of the new connection is set as IPv6. This causes cma_zero_addr
function to return true on an non-zero address. As a result, the wrong
code path is taken. This causes the connection request to be rejected,
as the RDMA-CM code looks for the wrong type of device.

Since copying the ip address is done in different function depending
on the family (cma_save_ip4_info/cma_save_ip6_info) this is fixed by
hard coding the family of the IP address according to the function.

Signed-off-by: Yotam Kenneth <yotamke@mellanox.com>
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
---
 drivers/infiniband/core/cma.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Jason Gunthorpe April 20, 2015, 4:41 p.m. UTC | #1 
On Mon, Apr 20, 2015 at 12:03:32PM +0300, Haggai Eran wrote:
> From: Yotam Kenneth <yotamke@mellanox.com>
> 
> When accepting a new connection with the listener being IPv6, the
> family of the new connection is set as IPv6. This causes cma_zero_addr
> function to return true on an non-zero address. As a result, the wrong
> code path is taken. This causes the connection request to be rejected,
> as the RDMA-CM code looks for the wrong type of device.

This description doesn't really make sense as to what the problem is.

> @@ -866,12 +866,12 @@ static void cma_save_ip4_info(struct rdma_cm_id *id, struct rdma_cm_id *listen_i
>  
>  	listen4 = (struct sockaddr_in *) &listen_id->route.addr.src_addr;
>  	ip4 = (struct sockaddr_in *) &id->route.addr.src_addr;
> -	ip4->sin_family = listen4->sin_family;
> +	ip4->sin_family = AF_INET;

If listen_id->route.addr.src_addr.ss_family != AF_INET then it is
invalid to cast to sockaddr_in.

So listen4->sin_family MUST be AF_INET or this function MUST NOT be
called.

Forcing to AF_INET cannot be correct here.

What does this patch have to do with this series?

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Or Gerlitz April 20, 2015, 6:38 p.m. UTC | #2 
On Mon, Apr 20, 2015 at 7:41 PM, Jason Gunthorpe
<jgunthorpe@obsidianresearch.com> wrote:
> On Mon, Apr 20, 2015 at 12:03:32PM +0300, Haggai Eran wrote:
>> From: Yotam Kenneth <yotamke@mellanox.com>
>>
>> When accepting a new connection with the listener being IPv6, the
>> family of the new connection is set as IPv6. This causes cma_zero_addr
>> function to return true on an non-zero address. As a result, the wrong
>> code path is taken. This causes the connection request to be rejected,
>> as the RDMA-CM code looks for the wrong type of device.
>
> This description doesn't really make sense as to what the problem is.
>
>> @@ -866,12 +866,12 @@ static void cma_save_ip4_info(struct rdma_cm_id *id, struct rdma_cm_id *listen_i
>>
>>       listen4 = (struct sockaddr_in *) &listen_id->route.addr.src_addr;
>>       ip4 = (struct sockaddr_in *) &id->route.addr.src_addr;
>> -     ip4->sin_family = listen4->sin_family;
>> +     ip4->sin_family = AF_INET;
>
> If listen_id->route.addr.src_addr.ss_family != AF_INET then it is
> invalid to cast to sockaddr_in.
>
> So listen4->sin_family MUST be AF_INET or this function MUST NOT be
> called.
>
> Forcing to AF_INET cannot be correct here.

Jason, could you take a look @ this thread
http://marc.info/?t=141589395000004&r=1&w=2 where the authors
addressed some comments from Sean and he eventually Acked the patch?

> What does this patch have to do with this series?

I believe it's either a pre-patch to address some assumption or
something they stepped on while testing

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Shachar Raindel April 21, 2015, 5:18 a.m. UTC | #3 
> -----Original Message-----

> From: Or Gerlitz [mailto:gerlitz.or@gmail.com]

> Sent: Monday, April 20, 2015 9:38 PM

> 

> On Mon, Apr 20, 2015 at 7:41 PM, Jason Gunthorpe

> <jgunthorpe@obsidianresearch.com> wrote:

> > On Mon, Apr 20, 2015 at 12:03:32PM +0300, Haggai Eran wrote:

> >> From: Yotam Kenneth <yotamke@mellanox.com>

> >>

> >> When accepting a new connection with the listener being IPv6, the

> >> family of the new connection is set as IPv6. This causes

> cma_zero_addr

> >> function to return true on an non-zero address. As a result, the

> wrong

> >> code path is taken. This causes the connection request to be

> rejected,

> >> as the RDMA-CM code looks for the wrong type of device.

> >

> > This description doesn't really make sense as to what the problem is.

> >

> >> @@ -866,12 +866,12 @@ static void cma_save_ip4_info(struct rdma_cm_id

> *id, struct rdma_cm_id *listen_i

> >>

> >>       listen4 = (struct sockaddr_in *) &listen_id-

> >route.addr.src_addr;

> >>       ip4 = (struct sockaddr_in *) &id->route.addr.src_addr;

> >> -     ip4->sin_family = listen4->sin_family;

> >> +     ip4->sin_family = AF_INET;

> >

> > If listen_id->route.addr.src_addr.ss_family != AF_INET then it is

> > invalid to cast to sockaddr_in.

> >

> > So listen4->sin_family MUST be AF_INET or this function MUST NOT be

> > called.

> >

> > Forcing to AF_INET cannot be correct here.

> 

> Jason, could you take a look @ this thread

> http://marc.info/?t=141589395000004&r=1&w=2 where the authors

> addressed some comments from Sean and he eventually Acked the patch?

> 

> > What does this patch have to do with this series?

> 

> I believe it's either a pre-patch to address some assumption or

> something they stepped on while testing

> 


We stepped upon this issue while testing the containers support we are
Submitting here. When creating a new network namespace, the kernel set 
net->ipv6.sysctl.bindv6only to 0. As a result, we got the IPv6 listening
ID accepting IPv4 connection. This is fixed by the above patch.

Thanks,
--Shachar
diff mbox

Patch

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index d570030d899c..6e5e11ca7702 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -866,12 +866,12 @@  static void cma_save_ip4_info(struct rdma_cm_id *id, struct rdma_cm_id *listen_i
 
 	listen4 = (struct sockaddr_in *) &listen_id->route.addr.src_addr;
 	ip4 = (struct sockaddr_in *) &id->route.addr.src_addr;
-	ip4->sin_family = listen4->sin_family;
+	ip4->sin_family = AF_INET;
 	ip4->sin_addr.s_addr = hdr->dst_addr.ip4.addr;
 	ip4->sin_port = listen4->sin_port;
 
 	ip4 = (struct sockaddr_in *) &id->route.addr.dst_addr;
-	ip4->sin_family = listen4->sin_family;
+	ip4->sin_family = AF_INET;
 	ip4->sin_addr.s_addr = hdr->src_addr.ip4.addr;
 	ip4->sin_port = hdr->port;
 }
@@ -883,12 +883,12 @@  static void cma_save_ip6_info(struct rdma_cm_id *id, struct rdma_cm_id *listen_i
 
 	listen6 = (struct sockaddr_in6 *) &listen_id->route.addr.src_addr;
 	ip6 = (struct sockaddr_in6 *) &id->route.addr.src_addr;
-	ip6->sin6_family = listen6->sin6_family;
+	ip6->sin6_family = AF_INET6;
 	ip6->sin6_addr = hdr->dst_addr.ip6;
 	ip6->sin6_port = listen6->sin6_port;
 
 	ip6 = (struct sockaddr_in6 *) &id->route.addr.dst_addr;
-	ip6->sin6_family = listen6->sin6_family;
+	ip6->sin6_family = AF_INET6;
 	ip6->sin6_addr = hdr->src_addr.ip6;
 	ip6->sin6_port = hdr->port;
 }