Messages in this thread | | | Subject | RE: [openib-general] [PATCH v2 1/7] AMSO1100 Low Level Driver. | From | Steve Wise <> | Date | Thu, 15 Jun 2006 09:03:31 -0500 |
| |
On Thu, 2006-06-15 at 08:41 -0500, Steve Wise wrote: > 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... >
Now that I've looked more into this, I'm not sure there's a simple way for the IWCM to copy the pdata on the upcall. Currently, the IWCM's event upcall, cm_event_handler(), simply queues the work for processing on a workqueue thread. So there's no per-event logic at all there. Lemme think on this more. Stay tuned.
Either way, the amso driver has a memory leak...
Steve.
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |