Thanks Alan for your comments.
On Monday 09 July 2012 08:04 PM, Alan Stern wrote:
On Mon, 9 Jul 2012, Venu Byravarasu wrote:
In functions itd_complete & sitd_complete, a pointer
by name stream may get dereferenced after freeing it, when
iso_stream_put is called with stream->refcount = 2.
I don't understand the problem. Did you actually see this happen or is
it only theoretical?
Yes it is a theoretical problem, as complained by Coverity.
/* for each uframe with a packet */
for (uframe = 0; uframe < 8; uframe++) {
@@ -1783,7 +1784,8 @@ itd_complete (
dev->devpath, stream->bEndpointAddress & 0x0f,
(stream->bEndpointAddress & USB_DIR_IN) ? "in" : "out");
}
- iso_stream_put (ehci, stream);
+ stream_ref_count = stream->refcount;
+ iso_stream_put(ehci, stream);
This iso_stream_put removes the reference held by the URB. Before it
is called, stream->refcount must be >= 3:
refcount is set to 1 when the stream is created;
each active URB holds a reference;
each itd holds a reference.
So after the call, the refcount value must be >= 2 and the stream could
not have been deallocated.
done:
itd->urb = NULL;
@@ -1797,7 +1799,7 @@ done:
* Move it to a safe place until a new frame starts.
*/
list_move(&itd->itd_list, &ehci->cached_itd_list);
- if (stream->refcount == 2) {
+ if (stream_ref_count == 3) {
Therefore this seems unnecessary.
As per the logic you explained above, this change is not needed.
However coverity was complaining as below:
/kernel/drivers/usb/host/ehci-sched.c 1777 USE_AFTER_FREE Dereferencing
freed pointer "stream"
Hence to pacify coverity, this change is done.
Please let me know if you see any other better way to handle it.
/* If iso_stream_put() were called here, stream
* would be freed. Instead, just prevent reuse.
*/
Alan Stern
Thanks,
Venu
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
