[PATCH v1] usb: host: Fix possible kernel crash

[PATCH v1] usb: host: Fix possible kernel crash

[Venu Byravarasu]

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.

Hence fixing it.

Signed-off-by: Venu Byravarasu <vbyravarasu@nvidia.com>
---
By mistake sent incorrect patch set number as v2 earlier.
Hence fixing it.

 drivers/usb/host/ehci-sched.c |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
index 33182c6..20d0c38 100644
--- a/drivers/usb/host/ehci-sched.c
+++ b/drivers/usb/host/ehci-sched.c
@@ -1715,6 +1715,7 @@ itd_complete (
 	struct ehci_iso_stream			*stream = itd->stream;
 	struct usb_device			*dev;
 	unsigned				retval = false;
+	u32					stream_ref_count = 0;
 
 	/* 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);
 
 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) {
 			/* If iso_stream_put() were called here, stream
 			 * would be freed.  Instead, just prevent reuse.
 			 */
@@ -1866,7 +1868,7 @@ done_not_linked:
 
 done:
 	if (unlikely (status < 0))
-		iso_stream_put (ehci, stream);
+		iso_stream_put(ehci, stream);
 	return status;
 }
 
@@ -2127,6 +2129,7 @@ sitd_complete (
 	struct ehci_iso_stream			*stream = sitd->stream;
 	struct usb_device			*dev;
 	unsigned				retval = false;
+	u32					stream_ref_count = 0;
 
 	urb_index = sitd->index;
 	desc = &urb->iso_frame_desc [urb_index];
@@ -2179,7 +2182,8 @@ sitd_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);
 
 done:
 	sitd->urb = NULL;
@@ -2193,7 +2197,7 @@ done:
 		 * Move it to a safe place until a new frame starts.
 		 */
 		list_move(&sitd->sitd_list, &ehci->cached_sitd_list);
-		if (stream->refcount == 2) {
+		if (stream_ref_count == 3) {
 			/* If iso_stream_put() were called here, stream
 			 * would be freed.  Instead, just prevent reuse.
 			 */
@@ -2259,7 +2263,7 @@ done_not_linked:
 
 done:
 	if (status < 0)
-		iso_stream_put (ehci, stream);
+		iso_stream_put(ehci, stream);
 	return status;
 }
 
-- 
1.7.1.1

Re: [PATCH v1] usb: host: Fix possible kernel crash

[Alan Stern]

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?

> Hence fixing it.
> 
> Signed-off-by: Venu Byravarasu <vbyravarasu@nvidia.com>
> ---
> By mistake sent incorrect patch set number as v2 earlier.
> Hence fixing it.
> 
>  drivers/usb/host/ehci-sched.c |   16 ++++++++++------
>  1 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
> index 33182c6..20d0c38 100644
> --- a/drivers/usb/host/ehci-sched.c
> +++ b/drivers/usb/host/ehci-sched.c
> @@ -1715,6 +1715,7 @@ itd_complete (
>  	struct ehci_iso_stream			*stream = itd->stream;
>  	struct usb_device			*dev;
>  	unsigned				retval = false;
> +	u32					stream_ref_count = 0;
>  
>  	/* 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.

>  			/* If iso_stream_put() were called here, stream
>  			 * would be freed.  Instead, just prevent reuse.
>  			 */

Alan Stern

Re: [PATCH v1] usb: host: Fix possible kernel crash

[Venu Byravarasu]

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

Re: [PATCH v1] usb: host: Fix possible kernel crash

[gregkh]

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

RE: [PATCH v1] usb: host: Fix possible kernel crash

[Scan Subscription]

Hi Greg/Venu/Alan and others,

The defect discussed in this thread was found in 2006, and was marked in Coverity Scan as false positive - intentional ( by linux developer or coverity admin that we don't know)...

As a general rule,
1. what was discussed with some of the Linux folks, Focus on NEW defects...
2. Do NOT fix anything that is already marked as Intentional /False Positive


For any defects found by Covierty Scan there could be potential 3 actions
1. Review and Fix the defect
2. Mark it as Intentional in Coverity Scan, and it will not appear as Defect in the future
3. Contact Coverity Scan-admin, if you would like to understand it more why it was flagged as defect.

• As we all know, inherent in the technology foundation, static analysis will report some false positives. We put a lot of effort into our product to drive this rate to a very low, acceptable limit (commonly between 5% and 25%)
• In order to address FPs, the SCAN part of our product offers a triage process that utilizes a persistent database based on defect hashing. This gives developers the option to declare a defect as FP and then never have to look at it again.

For instance, 3 weeks ago, Coverity reported 7 new defects introduced based on recent code changes,  and as we can see in the various threads almost everything was fixed and committed by maintainer.
But a  week after that, out of 6 new defects reported based on latest code change, 1 was ignored stating False positive, Intentional...

I hope this clarifies the issue that was discussed here.

Thanks

Coverity Scan-admin scan-admin@coverity.com 
Dakshesh Vyas | Technical Manager - SCAN
Coverity | 185 Berry Street | Suite 6500, Lobby 3 | San Francisco, CA  94107 
Office: 415.935.2957 | dvyas@coverity.com


________________________________________
From: linux-kernel-owner@vger.kernel.org [linux-kernel-owner@vger.kernel.org] On Behalf Of gregkh@linuxfoundation.org [gregkh@linuxfoundation.org]
Sent: Tuesday, July 10, 2012 7:45 AM
To: Venu Byravarasu
Cc: Alan Stern; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1] usb: host: Fix possible kernel crash

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
--
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/

Re: [PATCH v1] usb: host: Fix possible kernel crash

[Venu Byravarasu]

On Tuesday 10 July 2012 10:05 PM, Scan Subscription wrote:
> Hi Greg/Venu/Alan and others,
>
> The defect discussed in this thread was found in 2006, and was marked in Coverity Scan as false positive - intentional ( by linux developer or coverity admin that we don't know)...
>
> As a general rule,
> 1. what was discussed with some of the Linux folks, Focus on NEW defects...
> 2. Do NOT fix anything that is already marked as Intentional /False Positive
>
>
> For any defects found by Covierty Scan there could be potential 3 actions
> 1. Review and Fix the defect
> 2. Mark it as Intentional in Coverity Scan, and it will not appear as Defect in the future
> 3. Contact Coverity Scan-admin, if you would like to understand it more why it was flagged as defect.
>
> • As we all know, inherent in the technology foundation, static analysis will report some false positives. We put a lot of effort into our product to drive this rate to a very low, acceptable limit (commonly between 5% and 25%)
> • In order to address FPs, the SCAN part of our product offers a triage process that utilizes a persistent database based on defect hashing. This gives developers the option to declare a defect as FP and then never have to look at it again.
>
> For instance, 3 weeks ago, Coverity reported 7 new defects introduced based on recent code changes,  and as we can see in the various threads almost everything was fixed and committed by maintainer.
> But a  week after that, out of 6 new defects reported based on latest code change, 1 was ignored stating False positive, Intentional...
>
> I hope this clarifies the issue that was discussed here.

Thanks for your detailed mail.


> Thanks
>
> Coverity Scan-admin scan-admin@coverity.com
> Dakshesh Vyas | Technical Manager - SCAN
> Coverity | 185 Berry Street | Suite 6500, Lobby 3 | San Francisco, CA  94107
> Office: 415.935.2957 | dvyas@coverity.com
>
>
> ________________________________________
> From: linux-kernel-owner@vger.kernel.org [linux-kernel-owner@vger.kernel.org] On Behalf Of gregkh@linuxfoundation.org [gregkh@linuxfoundation.org]
> Sent: Tuesday, July 10, 2012 7:45 AM
> To: Venu Byravarasu
> Cc: Alan Stern; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v1] usb: host: Fix possible kernel crash
>
> 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
> --
> 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/

Re: [PATCH v1] usb: host: Fix possible kernel crash

[Venu Byravarasu]

On Tuesday 10 July 2012 08:15 PM, gregkh@linuxfoundation.org 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

Re: [PATCH v1] usb: host: Fix possible kernel crash

[Alan Stern]

On Tue, 10 Jul 2012, 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.

> 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.

This seems to be a false positive from Coverity.

In any case, I'm about to submit some patches which get rid of the 
reference counting entirely.  So let's not worry about this.

Alan Stern

Re: [PATCH v1] usb: host: Fix possible kernel crash

[Venu Byravarasu]

On Tuesday 10 July 2012 08:39 PM, Alan Stern wrote:
> On Tue, 10 Jul 2012, 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.
>> 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.
> This seems to be a false positive from Coverity.
>
> In any case, I'm about to submit some patches which get rid of the
> reference counting entirely.  So let's not worry about this.
>
> Alan Stern
>
Thanks Alan for taking care of it, in your future patch.