RE: [openib-general] [PATCH v2 1/7] AMSO1100 Low Level Driver.

On Wed, 2006-06-14 at 20:35 -0500, Bob Sharp wrote:

+void c2_ae_event(struct c2_dev *c2dev, u32 mq_index)
+{
+

<snip>

+ case C2_RES_IND_EP:{
+
+ struct c2wr_ae_connection_request *req =
+ &wr->ae.ae_connection_request;
+ struct iw_cm_id *cm_id =
+ (struct iw_cm_id *)resource_user_context;
+
+ pr_debug("C2_RES_IND_EP event_id=%d\n", event_id);
+ if (event_id != CCAE_CONNECTION_REQUEST) {
+ pr_debug("%s: Invalid event_id: %d\n",
+ __FUNCTION__, event_id);
+ break;
+ }
+ cm_event.event = IW_CM_EVENT_CONNECT_REQUEST;
+ cm_event.provider_data = (void*)(unsigned
long)req->cr_handle;
+ cm_event.local_addr.sin_addr.s_addr = req->laddr;
+ cm_event.remote_addr.sin_addr.s_addr = req->raddr;
+ cm_event.local_addr.sin_port = req->lport;
+ cm_event.remote_addr.sin_port = req->rport;
+ cm_event.private_data_len =
+ be32_to_cpu(req->private_data_length);
+
+ if (cm_event.private_data_len) {


It looks to me as if pdata is leaking here since it is not tracked and
the upper layers do not free it. Also, if pdata is freed after the call
to cm_id->event_handler returns, it exposes an issue in user space where
the private data is garbage. I suspect the iwarp cm should be copying
this data before it returns.


Good catch.

Yes, I think the IWCM should copy the private data in the upcall. If it
does, then the amso driver doesn't need to kmalloc()/copy at all. It
can pass a ptr to its MQ entry directly...

Thanks,

Steve.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Relevant Pages