On Tuesday 10 July 2012 08:15 PM, gregkh@xxxxxxxxxxxxxxxxxxx wrote:
On Tue, Jul 10, 2012 at 09:56:39AM +0530, Venu Byravarasu wrote:
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.
Why are you trying to "pacify" coverity, when the tool is wrong in this
case? Go poke the owners of that tool to get it to stop emitting this
false warning. Don't paper over it in the kernel. Especially for a
tool that none of us can run on our own.
greg k-h
Thanks Greg for your comments.
In fact coverity team also mentioned this as one of the false positives.
Also as Alan mentioned that he'll be taking care of it in a different
way, will stop working on this patch.
Thanks,
Venu
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
