[PATCH] usb: storage: realtek_cr: fix use-after-free on disconnect

[PATCH] usb: storage: realtek_cr: fix use-after-free on disconnect

[Myeonghun Pak]

realtek_cr_destructor() calls timer_delete() before the chip containing
the timer is freed. The timer callback may still be running and can
rearm itself, resulting in a use-after-free.

Use timer_shutdown_sync() to wait for the callback and prevent further
rearming. Do this unconditionally because ss_en may be changed after
the timer is armed.

Move timer_setup() into init_realtek_cr() so the timer is initialized
before any failure path can invoke the destructor.

Found by static analysis.

Fixes: e931830bb877 ("Realtek cr: Add autosuspend function.")
Cc: stable@vger.kernel.org
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
 drivers/usb/storage/realtek_cr.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/storage/realtek_cr.c b/drivers/usb/storage/realtek_cr.c
index af038b897c6b..c4b28744693b 100644
--- a/drivers/usb/storage/realtek_cr.c
+++ b/drivers/usb/storage/realtek_cr.c
@@ -916,7 +916,6 @@ static int realtek_cr_autosuspend_setup(struct us_data *us)
 	us->proto_handler = rts51x_invoke_transport;
 
 	chip->timer_expires = 0;
-	timer_setup(&chip->rts51x_suspend_timer, rts51x_suspend_timer_fn, 0);
 	fw5895_init(us);
 
 	/* enable autosuspend function of the usb device */
@@ -934,10 +933,7 @@ static void realtek_cr_destructor(void *extra)
 		return;
 
 #ifdef CONFIG_REALTEK_AUTOPM
-	if (ss_en) {
-		timer_delete(&chip->rts51x_suspend_timer);
-		chip->timer_expires = 0;
-	}
+	timer_shutdown_sync(&chip->rts51x_suspend_timer);
 #endif
 	kfree(chip->status);
 }
@@ -982,6 +978,9 @@ static int init_realtek_cr(struct us_data *us)
 
 	us->extra = chip;
 	us->extra_destructor = realtek_cr_destructor;
+#ifdef CONFIG_REALTEK_AUTOPM
+	timer_setup(&chip->rts51x_suspend_timer, rts51x_suspend_timer_fn, 0);
+#endif
 	us->max_lun = chip->max_lun = rts51x_get_max_lun(us);
 	chip->us = us;
 
-- 
2.47.1

Re: [PATCH] usb: storage: realtek_cr: fix use-after-free on disconnect

[Alan Stern]

On Mon, Jul 27, 2026 at 09:34:14PM +0900, Myeonghun Pak wrote:
> realtek_cr_destructor() calls timer_delete() before the chip containing
> the timer is freed. The timer callback may still be running and can
> rearm itself, resulting in a use-after-free.
> 
> Use timer_shutdown_sync() to wait for the callback and prevent further
> rearming. Do this unconditionally because ss_en may be changed after
> the timer is armed.
> 
> Move timer_setup() into init_realtek_cr() so the timer is initialized
> before any failure path can invoke the destructor.
> 
> Found by static analysis.
> 
> Fixes: e931830bb877 ("Realtek cr: Add autosuspend function.")
> Cc: stable@vger.kernel.org
> Co-developed-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
> ---

This is fine as far as it goes, but the real question is why realtek_cr 
uses this elaborate suspend machinery instead of just relying on normal 
runtime PM.

This material was added in commit e931830bb877e2aa ("Realtek cr: Add 
autosuspend function.") with no explanation about why it was so 
complicated.  Can anyone say why all this complication shouldn't be 
removed?

Alan Stern

>  drivers/usb/storage/realtek_cr.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/storage/realtek_cr.c b/drivers/usb/storage/realtek_cr.c
> index af038b897c6b..c4b28744693b 100644
> --- a/drivers/usb/storage/realtek_cr.c
> +++ b/drivers/usb/storage/realtek_cr.c
> @@ -916,7 +916,6 @@ static int realtek_cr_autosuspend_setup(struct us_data *us)
>  	us->proto_handler = rts51x_invoke_transport;
>  
>  	chip->timer_expires = 0;
> -	timer_setup(&chip->rts51x_suspend_timer, rts51x_suspend_timer_fn, 0);
>  	fw5895_init(us);
>  
>  	/* enable autosuspend function of the usb device */
> @@ -934,10 +933,7 @@ static void realtek_cr_destructor(void *extra)
>  		return;
>  
>  #ifdef CONFIG_REALTEK_AUTOPM
> -	if (ss_en) {
> -		timer_delete(&chip->rts51x_suspend_timer);
> -		chip->timer_expires = 0;
> -	}
> +	timer_shutdown_sync(&chip->rts51x_suspend_timer);
>  #endif
>  	kfree(chip->status);
>  }
> @@ -982,6 +978,9 @@ static int init_realtek_cr(struct us_data *us)
>  
>  	us->extra = chip;
>  	us->extra_destructor = realtek_cr_destructor;
> +#ifdef CONFIG_REALTEK_AUTOPM
> +	timer_setup(&chip->rts51x_suspend_timer, rts51x_suspend_timer_fn, 0);
> +#endif
>  	us->max_lun = chip->max_lun = rts51x_get_max_lun(us);
>  	chip->us = us;
>  
> -- 
> 2.47.1
>