On Mar 26, 2009, at 3:43 AM, Greg Banks wrote:
Chuck Lever wrote:
On Mar 24, 2009, at 4:32 PM, J. Bruce Fields wrote:
@@ -563,9 +564,20 @@ static void handle_connect_req(struct
rdma_cm_id *new_cma_id, size_t client_ird)
/* Set the local and remote addresses in the transport */
sa = (struct sockaddr *)&newxprt->sc_cm_id->route.addr.dst_addr;
- svc_xprt_set_remote(&newxprt->sc_xprt, sa, svc_addr_len(sa));
+ len = svc_addr_len(sa);
+ if (len < 0) {
+ dprintk("svcrdma: dst_addr has a bad address family\n");
+ return;
we're probably leaking something here.
Just a bit. The svcxprt_rdma, the rdma_cm_id, a bunch of other IB
state...
I don't want to fix this until it's understood well enough to fix it
correctly.
Tom needs respond to this, but I think it would be safe to simply
BUG() here.
My 2c. If I understand the RDMA CM behaviour correctly, it should
not be
possible at this point in the code for
newxprt->sc_cm_id->route.addr.{src,dst}_addr to be anything but a
valid
sockaddr_storages holding the IPv4 or (theoretically) the IPv6 address
of the server and client. So I would be happy with a BUG_ON(len < 0).
That would also render the leakage moot.
My primary objection is the negative return value from
svc_addr_len(). Perhaps it would be safer all around if
svc_addr_len() returned zero if it didn't recognize the address family
in the passed in sockaddr.
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
