Re: [PATCH 2/3] USB: r8a66597-hcd: fix driver removing

Alan Stern wrote:
> On Wed, 3 Oct 2007, Yoshihiro Shimoda wrote:
>
>> Fixed the problem that accessed register of this controller after
>> having called iounmap().
>>
>> Signed-off-by: Yoshihiro Shimoda <[email protected]>
>> ---
>>  drivers/usb/host/r8a66597-hcd.c |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff -uprN a/drivers/usb/host/r8a66597-hcd.c b/drivers/usb/host/r8a66597-hcd.c
>> --- a/drivers/usb/host/r8a66597-hcd.c	2007-10-03 11:24:37.000000000 +0900
>> +++ b/drivers/usb/host/r8a66597-hcd.c	2007-10-03 11:42:06.000000000 +0900
>> @@ -2126,9 +2126,9 @@ static int __init_or_module r8a66597_rem
>>  	struct usb_hcd		*hcd = r8a66597_to_hcd(r8a66597);
>>
>>  	del_timer_sync(&r8a66597->rh_timer);
>> -	iounmap((void *)r8a66597->reg);
>>  	usb_remove_hcd(hcd);
>>  	usb_put_hcd(hcd);
>> +	iounmap((void *)r8a66597->reg);
>>  	return 0;
>>  }
>
> This is wrong.  After usb_put_hcd(hcd), r8a66597 points to memory that
> may have been freed.  The iounmap call has to come before
> usb_put_hcd(), but it could come after usb_remove_hcd().

Thank you very much for your comment.
I understand it.

> Why do you see register accesses after the unmapping?  At this point in
> the driver, nothing should be trying to access the controller hardware.

Because r8a66597_stop() is called by hcd->driver->stop() in usb_remove_hcd().
And because r8a66597_stop() call disable_controller(), original code
accessed register after having called iounmap().

I made a new patch. I would appreciate it if you could check this patch.

Thanks,
Yoshihiro Shimoda

---
diff -uprN a/drivers/usb/host/r8a66597-hcd.c b/drivers/usb/host/r8a66597-hcd.c
--- a/drivers/usb/host/r8a66597-hcd.c	2007-10-04 10:34:58.000000000 +0900
+++ b/drivers/usb/host/r8a66597-hcd.c	2007-10-04 10:40:20.000000000 +0900
@@ -2126,8 +2126,8 @@ static int __init_or_module r8a66597_rem
 	struct usb_hcd		*hcd = r8a66597_to_hcd(r8a66597);

 	del_timer_sync(&r8a66597->rh_timer);
-	iounmap((void *)r8a66597->reg);
 	usb_remove_hcd(hcd);
+	iounmap((void *)r8a66597->reg);
 	usb_put_hcd(hcd);
 	return 0;
 }

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel