[PATCH 5/9] ib/addr: verify source and destination address families match

[PATCH 5/9] ib/addr: verify source and destination address families match

[Sean Hefty]

rdma_resolve_ip is never called with a NULL src_addr, so we just need
to verify that the source and destination address families match.

Signed-of-by: Sean Hefty <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---

 drivers/infiniband/core/addr.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/core/addr.c b/drivers/infiniband/core/addr.c
index ccc0f91..f5baf0b 100644
--- a/drivers/infiniband/core/addr.c
+++ b/drivers/infiniband/core/addr.c
@@ -461,8 +461,10 @@ int rdma_resolve_ip(struct rdma_addr_client *client,
 	if (!req)
 		return -ENOMEM;
 
-	if (src_addr)
-		memcpy(&req->src_addr, src_addr, ip_addr_size(src_addr));
+	if (src_addr->sa_family != dst_addr->sa_family)
+		return -EINVAL;
+
+	memcpy(&req->src_addr, src_addr, ip_addr_size(src_addr));
 	memcpy(&req->dst_addr, dst_addr, ip_addr_size(dst_addr));
 	req->addr = addr;
 	req->callback = callback;



--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/9 v2] ib/addr: verify source and destination address families match

[Sean Hefty]

ib/addr: verify source and destination address families match

From: Sean Hefty <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Signed-off-by: Sean Hefty <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
change from v1:
spell 'off' correctly in signed-off-by line

 drivers/infiniband/core/addr.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/core/addr.c b/drivers/infiniband/core/addr.c
index 788a02e..24c010b 100644
--- a/drivers/infiniband/core/addr.c
+++ b/drivers/infiniband/core/addr.c
@@ -461,8 +461,10 @@ int rdma_resolve_ip(struct rdma_addr_client *client,
 	if (!req)
 		return -ENOMEM;
 
-	if (src_addr)
-		memcpy(&req->src_addr, src_addr, ip_addr_size(src_addr));
+	if (src_addr->sa_family != dst_addr->sa_family)
+		return -EINVAL;
+
+	memcpy(&req->src_addr, src_addr, ip_addr_size(src_addr));
 	memcpy(&req->dst_addr, dst_addr, ip_addr_size(dst_addr));
 	req->addr = addr;
 	req->callback = callback;



--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/9 v2] ib/addr: verify source and destination address families match

[Roland Dreier]

 > -	if (src_addr)
 > -		memcpy(&req->src_addr, src_addr, ip_addr_size(src_addr));
 > +	if (src_addr->sa_family != dst_addr->sa_family)
 > +		return -EINVAL;
 
The old code seemed to allow for the possibility of src_addr being
NULL.  There only seems to be one in-tree caller of rdma_resolve_ip,
which does pass in a src_addr, but the API definition in the header says

 * @src_addr: An optional source address to use in the resolution.  If a

so it seems passing in NULL would be allowed?

 - R.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 5/9 v2] ib/addr: verify source and destination address families match

[Sean Hefty]

> > -	if (src_addr)
> > -		memcpy(&req->src_addr, src_addr, ip_addr_size(src_addr));
> > +	if (src_addr->sa_family != dst_addr->sa_family)
> > +		return -EINVAL;
>
>The old code seemed to allow for the possibility of src_addr being
>NULL.  There only seems to be one in-tree caller of rdma_resolve_ip,
>which does pass in a src_addr, but the API definition in the header says
>
> * @src_addr: An optional source address to use in the resolution.  If a
>
>so it seems passing in NULL would be allowed?

I forgot about the header file comments.  (I only though about rdma_resolve_addr
supporting a NULL src_addr.)  I took the check out when I noticed that src_addr
was always coming in non-NULL.  The address itself doesn't need to be set beyond
the address family, so it's still optional in that regard.  (A pretty weak
argument, but it's all I have. :)

I can go either way here - either update the comments in the header file, or
allow passing in NULL.  Do you or does anyone else have a preference?

- Sean

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/9 v2] ib/addr: verify source and destination address families match

[Roland Dreier]

 > I can go either way here - either update the comments in the header file, or
 > allow passing in NULL.  Do you or does anyone else have a preference?

I don't have a strong preference.  Or really any preference, except
maybe generally favoring interface stability -- ie keep doing what we've
been doing and leave src_addr as optional.  OTOH we could kill the
never-triggered test for src_addr and follow experience with the
interface.  Basically I don't know... whatever you'd rather do is fine.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/9 v3] ib/addr: verify source and destination address families match

[Sean Hefty]

If a source address is provided, verify that the address family matches
that of the destination address.  If the source is not specified, use the
same address family as the destination.

Signed-of-by: Sean Hefty <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
change from v2:
Continue to support src_addr being NULL.
Fix memory leak introduced by first patch.  Free req on failure.

change from v1:
spell 'off' correctly in signed-off-by line

 drivers/infiniband/core/addr.c |   27 +++++++++++++++++++--------
 1 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/drivers/infiniband/core/addr.c b/drivers/infiniband/core/addr.c
index 788a02e..b59ba7c 100644
--- a/drivers/infiniband/core/addr.c
+++ b/drivers/infiniband/core/addr.c
@@ -461,18 +461,27 @@ int rdma_resolve_ip(struct rdma_addr_client *client,
 	if (!req)
 		return -ENOMEM;
 
-	if (src_addr)
-		memcpy(&req->src_addr, src_addr, ip_addr_size(src_addr));
-	memcpy(&req->dst_addr, dst_addr, ip_addr_size(dst_addr));
+	src_in = (struct sockaddr *) &req->src_addr;
+	dst_in = (struct sockaddr *) &req->dst_addr;
+
+	if (src_addr) {
+		if (src_addr->sa_family != dst_addr->sa_family) {
+			ret = -EINVAL;
+			goto err;
+		}
+
+		memcpy(src_in, src_addr, ip_addr_size(src_addr));
+	} else {
+		src_in->sa_family = dst_addr->sa_family;
+	}
+
+	memcpy(dst_in, dst_addr, ip_addr_size(dst_addr));
 	req->addr = addr;
 	req->callback = callback;
 	req->context = context;
 	req->client = client;
 	atomic_inc(&client->refcount);
 
-	src_in = (struct sockaddr *) &req->src_addr;
-	dst_in = (struct sockaddr *) &req->dst_addr;
-
 	req->status = addr_resolve_local(src_in, dst_in, addr);
 	if (req->status == -EADDRNOTAVAIL)
 		req->status = addr_resolve_remote(src_in, dst_in, addr);
@@ -490,10 +499,12 @@ int rdma_resolve_ip(struct rdma_addr_client *client,
 	default:
 		ret = req->status;
 		atomic_dec(&client->refcount);
-		kfree(req);
-		break;
+		goto err;
 	}
 	return ret;
+err:
+	kfree(req);
+	return ret;
 }
 EXPORT_SYMBOL(rdma_resolve_ip);
 



--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html