[PATCH] memstick: core: reclaim the request before freeing a timed-out card

[PATCH] memstick: core: reclaim the request before freeing a timed-out card

[Nguyen Ngoc Thang]

memstick_alloc_card() hands card->current_mrq to the host and waits 500 ms
for it. If the host is still busy, the wait times out, the card is freed,
but the host keeps its pointer to the freed request.

rtsx_usb_ms hits this easily: its handle_req work can block in USB
transfers for seconds. When it returns, it reads and writes the freed
request, including the retry path in memstick_next_req():

  BUG: KASAN: slab-use-after-free in rtsx_usb_ms_handle_req+0x17ff/0x1a00
  Read of size 1 by task kworker/1:3
  Workqueue: events rtsx_usb_ms_handle_req
  Allocated by task 1656:
   memstick_alloc_card
   memstick_check
  Freed by task 1656:
   memstick_alloc_card
   memstick_check

Add an optional host->cancel() hook that stops the host from using the
current request, and call it from a common wait helper on timeout, before
the request's owner can go away. rtsx_usb_ms implements it by draining its
work item. The helper also covers memstick_set_rw_addr() and both waits in
memstick_alloc_card().

Reported-by: syzbot+3ee5da0319ca17ef1f4e@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=3ee5da0319ca17ef1f4e
Fixes: 99451dceeb5f ("memstick: Add realtek USB memstick host driver")
Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
---
Testing: no hardware. A raw-gadget emulation of an RTS5129 (0bda:0129) on
dummy_hcd delays every bulk-IN reply by 550 ms (above the core's 500 ms wait,
below the driver's 600 ms USB timeout). On a KASAN kernel the unpatched tree
reproduces the report (same offset and alloc/free stacks); the patched tree
takes the timeout+cancel path (~0.6 s) with no KASAN.

I first considered a second unbounded wait_for_completion() instead of a hook,
but rtsx_usb_ms_request() skips scheduling once host->eject is set and
memstick_remove_host() flushes the workqueue, so that can deadlock.

Known limit: cancel waits for the worker's remaining retries (a few seconds
with a device that never answers) while memstick_check() holds host->lock.
Other hosts leave ->cancel NULL, so their behaviour is unchanged.

 drivers/memstick/core/memstick.c    | 22 ++++++++++++++++------
 drivers/memstick/host/rtsx_usb_ms.c |  8 ++++++++
 include/linux/memstick.h            |  2 ++
 3 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/drivers/memstick/core/memstick.c b/drivers/memstick/core/memstick.c
index e03989c4e99e..ea09a63286bb 100644
--- a/drivers/memstick/core/memstick.c
+++ b/drivers/memstick/core/memstick.c
@@ -359,6 +359,20 @@ static int h_memstick_set_rw_addr(struct memstick_dev *card,
 	}
 }
 
+/* On timeout the host may still hold current_mrq; make it let go first. */
+static void memstick_wait_req(struct memstick_dev *card)
+{
+	struct memstick_host *host = card->host;
+
+	if (wait_for_completion_timeout(&card->mrq_complete,
+					msecs_to_jiffies(500)))
+		return;
+
+	if (host->cancel)
+		host->cancel(host);
+	card->current_mrq.error = -ETIMEDOUT;
+}
+
 /**
  * memstick_set_rw_addr - issue SET_RW_REG_ADDR request and wait for it to
  *                        complete
@@ -370,9 +384,7 @@ int memstick_set_rw_addr(struct memstick_dev *card)
 {
 	card->next_request = h_memstick_set_rw_addr;
 	memstick_new_req(card->host);
-	if (!wait_for_completion_timeout(&card->mrq_complete,
-			msecs_to_jiffies(500)))
-		card->current_mrq.error = -ETIMEDOUT;
+	memstick_wait_req(card);
 
 	return card->current_mrq.error;
 }
@@ -405,9 +417,7 @@ static struct memstick_dev *memstick_alloc_card(struct memstick_host *host)
 
 		card->next_request = h_memstick_read_dev_id;
 		memstick_new_req(host);
-		if (!wait_for_completion_timeout(&card->mrq_complete,
-				msecs_to_jiffies(500)))
-			card->current_mrq.error = -ETIMEDOUT;
+		memstick_wait_req(card);
 
 		if (card->current_mrq.error)
 			goto err_out;
diff --git a/drivers/memstick/host/rtsx_usb_ms.c b/drivers/memstick/host/rtsx_usb_ms.c
index beadc389f15f..403144a39a53 100644
--- a/drivers/memstick/host/rtsx_usb_ms.c
+++ b/drivers/memstick/host/rtsx_usb_ms.c
@@ -551,6 +551,13 @@ static void rtsx_usb_ms_request(struct memstick_host *msh)
 		schedule_work(&host->handle_req);
 }
 
+static void rtsx_usb_ms_cancel(struct memstick_host *msh)
+{
+	struct rtsx_usb_ms *host = memstick_priv(msh);
+
+	cancel_work_sync(&host->handle_req);
+}
+
 static int rtsx_usb_ms_set_param(struct memstick_host *msh,
 		enum memstick_param param, int value)
 {
@@ -787,6 +794,7 @@ static int rtsx_usb_ms_drv_probe(struct platform_device *pdev)
 	INIT_DELAYED_WORK(&host->poll_card, rtsx_usb_ms_poll_card);
 
 	msh->request = rtsx_usb_ms_request;
+	msh->cancel = rtsx_usb_ms_cancel;
 	msh->set_param = rtsx_usb_ms_set_param;
 	msh->caps = MEMSTICK_CAP_PAR4;
 
diff --git a/include/linux/memstick.h b/include/linux/memstick.h
index 107bdcbedf79..e86f7f6e3e4b 100644
--- a/include/linux/memstick.h
+++ b/include/linux/memstick.h
@@ -285,6 +285,8 @@ struct memstick_host {
 
 	/* Notify the host that some requests are pending. */
 	void                (*request)(struct memstick_host *host);
+	/* Stop using the current request; may sleep until the host is idle. */
+	void                (*cancel)(struct memstick_host *host);
 	/* Set host IO parameters (power, clock, etc).     */
 	int                 (*set_param)(struct memstick_host *host,
 					 enum memstick_param param,
-- 
2.43.0

Re: [PATCH] memstick: core: reclaim the request before freeing a timed-out card

[Ulf Hansson]

On Sat, Sep 19, 2026 at 12:06 PM Nguyen Ngoc Thang
<ngocthang2710.1999@gmail.com> wrote:
>
> memstick_alloc_card() hands card->current_mrq to the host and waits 500 ms
> for it. If the host is still busy, the wait times out, the card is freed,
> but the host keeps its pointer to the freed request.
>
> rtsx_usb_ms hits this easily: its handle_req work can block in USB
> transfers for seconds. When it returns, it reads and writes the freed
> request, including the retry path in memstick_next_req():
>
>   BUG: KASAN: slab-use-after-free in rtsx_usb_ms_handle_req+0x17ff/0x1a00
>   Read of size 1 by task kworker/1:3
>   Workqueue: events rtsx_usb_ms_handle_req
>   Allocated by task 1656:
>    memstick_alloc_card
>    memstick_check
>   Freed by task 1656:
>    memstick_alloc_card
>    memstick_check
>
> Add an optional host->cancel() hook that stops the host from using the
> current request, and call it from a common wait helper on timeout, before
> the request's owner can go away. rtsx_usb_ms implements it by draining its
> work item. The helper also covers memstick_set_rw_addr() and both waits in
> memstick_alloc_card().
>
> Reported-by: syzbot+3ee5da0319ca17ef1f4e@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=3ee5da0319ca17ef1f4e
> Fixes: 99451dceeb5f ("memstick: Add realtek USB memstick host driver")
> Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
> ---
> Testing: no hardware. A raw-gadget emulation of an RTS5129 (0bda:0129) on
> dummy_hcd delays every bulk-IN reply by 550 ms (above the core's 500 ms wait,
> below the driver's 600 ms USB timeout). On a KASAN kernel the unpatched tree
> reproduces the report (same offset and alloc/free stacks); the patched tree
> takes the timeout+cancel path (~0.6 s) with no KASAN.
>
> I first considered a second unbounded wait_for_completion() instead of a hook,
> but rtsx_usb_ms_request() skips scheduling once host->eject is set and
> memstick_remove_host() flushes the workqueue, so that can deadlock.
>
> Known limit: cancel waits for the worker's remaining retries (a few seconds
> with a device that never answers) while memstick_check() holds host->lock.
> Other hosts leave ->cancel NULL, so their behaviour is unchanged.
>
>  drivers/memstick/core/memstick.c    | 22 ++++++++++++++++------
>  drivers/memstick/host/rtsx_usb_ms.c |  8 ++++++++
>  include/linux/memstick.h            |  2 ++
>  3 files changed, 26 insertions(+), 6 deletions(-)

May I suggest that you split this into separate patches. For the core
and for the rtsx_usb driver. Other than that, this seems reasonable to
me.

In fact, we should probably have something similar for mmc, as
currently it's the mmc host driver responsibility to manage this
timeout itself.

Kind regards
Uffe

>
> diff --git a/drivers/memstick/core/memstick.c b/drivers/memstick/core/memstick.c
> index e03989c4e99e..ea09a63286bb 100644
> --- a/drivers/memstick/core/memstick.c
> +++ b/drivers/memstick/core/memstick.c
> @@ -359,6 +359,20 @@ static int h_memstick_set_rw_addr(struct memstick_dev *card,
>         }
>  }
>
> +/* On timeout the host may still hold current_mrq; make it let go first. */
> +static void memstick_wait_req(struct memstick_dev *card)
> +{
> +       struct memstick_host *host = card->host;
> +
> +       if (wait_for_completion_timeout(&card->mrq_complete,
> +                                       msecs_to_jiffies(500)))
> +               return;
> +
> +       if (host->cancel)
> +               host->cancel(host);
> +       card->current_mrq.error = -ETIMEDOUT;
> +}
> +
>  /**
>   * memstick_set_rw_addr - issue SET_RW_REG_ADDR request and wait for it to
>   *                        complete
> @@ -370,9 +384,7 @@ int memstick_set_rw_addr(struct memstick_dev *card)
>  {
>         card->next_request = h_memstick_set_rw_addr;
>         memstick_new_req(card->host);
> -       if (!wait_for_completion_timeout(&card->mrq_complete,
> -                       msecs_to_jiffies(500)))
> -               card->current_mrq.error = -ETIMEDOUT;
> +       memstick_wait_req(card);
>
>         return card->current_mrq.error;
>  }
> @@ -405,9 +417,7 @@ static struct memstick_dev *memstick_alloc_card(struct memstick_host *host)
>
>                 card->next_request = h_memstick_read_dev_id;
>                 memstick_new_req(host);
> -               if (!wait_for_completion_timeout(&card->mrq_complete,
> -                               msecs_to_jiffies(500)))
> -                       card->current_mrq.error = -ETIMEDOUT;
> +               memstick_wait_req(card);
>
>                 if (card->current_mrq.error)
>                         goto err_out;
> diff --git a/drivers/memstick/host/rtsx_usb_ms.c b/drivers/memstick/host/rtsx_usb_ms.c
> index beadc389f15f..403144a39a53 100644
> --- a/drivers/memstick/host/rtsx_usb_ms.c
> +++ b/drivers/memstick/host/rtsx_usb_ms.c
> @@ -551,6 +551,13 @@ static void rtsx_usb_ms_request(struct memstick_host *msh)
>                 schedule_work(&host->handle_req);
>  }
>
> +static void rtsx_usb_ms_cancel(struct memstick_host *msh)
> +{
> +       struct rtsx_usb_ms *host = memstick_priv(msh);
> +
> +       cancel_work_sync(&host->handle_req);
> +}
> +
>  static int rtsx_usb_ms_set_param(struct memstick_host *msh,
>                 enum memstick_param param, int value)
>  {
> @@ -787,6 +794,7 @@ static int rtsx_usb_ms_drv_probe(struct platform_device *pdev)
>         INIT_DELAYED_WORK(&host->poll_card, rtsx_usb_ms_poll_card);
>
>         msh->request = rtsx_usb_ms_request;
> +       msh->cancel = rtsx_usb_ms_cancel;
>         msh->set_param = rtsx_usb_ms_set_param;
>         msh->caps = MEMSTICK_CAP_PAR4;
>
> diff --git a/include/linux/memstick.h b/include/linux/memstick.h
> index 107bdcbedf79..e86f7f6e3e4b 100644
> --- a/include/linux/memstick.h
> +++ b/include/linux/memstick.h
> @@ -285,6 +285,8 @@ struct memstick_host {
>
>         /* Notify the host that some requests are pending. */
>         void                (*request)(struct memstick_host *host);
> +       /* Stop using the current request; may sleep until the host is idle. */
> +       void                (*cancel)(struct memstick_host *host);
>         /* Set host IO parameters (power, clock, etc).     */
>         int                 (*set_param)(struct memstick_host *host,
>                                          enum memstick_param param,
> --
> 2.43.0
>

Re: [PATCH] memstick: core: reclaim the request before freeing a timed-out card

[Ulf Hansson]

On Tue, Sep 29, 2026 at 12:24 PM Ulf Hansson
<ulf.hansson@oss.qualcomm.com> wrote:
>
> On Sat, Sep 19, 2026 at 12:06 PM Nguyen Ngoc Thang
> <ngocthang2710.1999@gmail.com> wrote:
> >
> > memstick_alloc_card() hands card->current_mrq to the host and waits 500 ms
> > for it. If the host is still busy, the wait times out, the card is freed,
> > but the host keeps its pointer to the freed request.
> >
> > rtsx_usb_ms hits this easily: its handle_req work can block in USB
> > transfers for seconds. When it returns, it reads and writes the freed
> > request, including the retry path in memstick_next_req():
> >
> >   BUG: KASAN: slab-use-after-free in rtsx_usb_ms_handle_req+0x17ff/0x1a00
> >   Read of size 1 by task kworker/1:3
> >   Workqueue: events rtsx_usb_ms_handle_req
> >   Allocated by task 1656:
> >    memstick_alloc_card
> >    memstick_check
> >   Freed by task 1656:
> >    memstick_alloc_card
> >    memstick_check
> >
> > Add an optional host->cancel() hook that stops the host from using the
> > current request, and call it from a common wait helper on timeout, before
> > the request's owner can go away. rtsx_usb_ms implements it by draining its
> > work item. The helper also covers memstick_set_rw_addr() and both waits in
> > memstick_alloc_card().
> >
> > Reported-by: syzbot+3ee5da0319ca17ef1f4e@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=3ee5da0319ca17ef1f4e
> > Fixes: 99451dceeb5f ("memstick: Add realtek USB memstick host driver")
> > Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
> > ---
> > Testing: no hardware. A raw-gadget emulation of an RTS5129 (0bda:0129) on
> > dummy_hcd delays every bulk-IN reply by 550 ms (above the core's 500 ms wait,
> > below the driver's 600 ms USB timeout). On a KASAN kernel the unpatched tree
> > reproduces the report (same offset and alloc/free stacks); the patched tree
> > takes the timeout+cancel path (~0.6 s) with no KASAN.
> >
> > I first considered a second unbounded wait_for_completion() instead of a hook,
> > but rtsx_usb_ms_request() skips scheduling once host->eject is set and
> > memstick_remove_host() flushes the workqueue, so that can deadlock.
> >
> > Known limit: cancel waits for the worker's remaining retries (a few seconds
> > with a device that never answers) while memstick_check() holds host->lock.
> > Other hosts leave ->cancel NULL, so their behaviour is unchanged.
> >
> >  drivers/memstick/core/memstick.c    | 22 ++++++++++++++++------
> >  drivers/memstick/host/rtsx_usb_ms.c |  8 ++++++++
> >  include/linux/memstick.h            |  2 ++
> >  3 files changed, 26 insertions(+), 6 deletions(-)
>
> May I suggest that you split this into separate patches. For the core
> and for the rtsx_usb driver. Other than that, this seems reasonable to
> me.
>
> In fact, we should probably have something similar for mmc, as
> currently it's the mmc host driver responsibility to manage this
> timeout itself.

Reviewing another patch [1] for the same issue, indicates the memstick
host drivers are already managing the timeout themselves. So, even if
your approach seems reasonable, I decided to go with the other
solution for now.

[...]

Kind regards
Uffe

[1]
https://lore.kernel.org/all/20260924204142.607-1-rajojha047@gmail.com/

Re: [PATCH] memstick: core: reclaim the request before freeing a timed-out card

[Nguyen Ngoc Thang]

On Tue, Sep 29, 2026 at 12:57 PM Ulf Hansson wrote:
> Reviewing another patch [1] for the same issue, indicates the memstick
> host drivers are already managing the timeout themselves. So, even if
> your approach seems reasonable, I decided to go with the other
> solution for now.

Fine with me. Raj's patch fixes the UAF in my reproducer as well.

One concern: it is effectively a revert of b65e630a55a4 ("memstick: Add
timeout to prevent indefinite waiting"), which was the backstop for the
rtsx_usb_ms remove race that 99d7ab8db9d8 ("memstick: Fix deadlock by
moving removing flag earlier") only narrowed.

memstick_check() can still pass the host->removing check just before
rtsx_usb_ms_drv_remove() sets eject/removing. Its next request then
reaches rtsx_usb_ms_request(), which drops it because eject is set, so
nobody completes mrq_complete. With an unbounded wait, memstick_check()
never returns and memstick_remove_host() blocks in flush_workqueue()
forever. (cancel_work_sync() in drv_remove cancelling a queued
handle_req before it picks up the request looks like another way to
get there.)

I reproduced it in QEMU with dummy_hcd + raw-gadget emulating an
RTS5129, widening the window with a debug msleep() right after the
host->removing check and unplugging the device during it:

  INFO: task kworker/u10:3:65 blocked for more than 20 seconds.
  Workqueue: kmemstick memstick_check
   __wait_for_common
   memstick_check

  INFO: task kworker/1:1:33 blocked for more than 20 seconds.
  Workqueue: usb_hub_wq hub_event
   __flush_workqueue
   memstick_remove_host
   rtsx_usb_ms_drv_remove
   platform_remove
   ...
   rtsx_usb_disconnect
   usb_disconnect
   hub_event

The same test on mainline (500 ms timeout) recovers fine.

Since memstick hosts are expected to always complete a request, I think
the right follow-up is in rtsx_usb_ms: complete requests with an error
after eject instead of dropping them. I can send that as a separate
rtsx_usb_ms patch on top of Raj's, if that works for you.

Thanks,
Thang

Re: [PATCH] memstick: core: reclaim the request before freeing a timed-out card

[Ulf Hansson]

On Tue, Sep 29, 2026 at 6:12 PM Nguyen Ngoc Thang
<ngocthang2710.1999@gmail.com> wrote:
>
> On Tue, Sep 29, 2026 at 12:57 PM Ulf Hansson wrote:
> > Reviewing another patch [1] for the same issue, indicates the memstick
> > host drivers are already managing the timeout themselves. So, even if
> > your approach seems reasonable, I decided to go with the other
> > solution for now.
>
> Fine with me. Raj's patch fixes the UAF in my reproducer as well.
>
> One concern: it is effectively a revert of b65e630a55a4 ("memstick: Add
> timeout to prevent indefinite waiting"), which was the backstop for the
> rtsx_usb_ms remove race that 99d7ab8db9d8 ("memstick: Fix deadlock by
> moving removing flag earlier") only narrowed.
>
> memstick_check() can still pass the host->removing check just before
> rtsx_usb_ms_drv_remove() sets eject/removing. Its next request then
> reaches rtsx_usb_ms_request(), which drops it because eject is set, so
> nobody completes mrq_complete. With an unbounded wait, memstick_check()
> never returns and memstick_remove_host() blocks in flush_workqueue()
> forever. (cancel_work_sync() in drv_remove cancelling a queued
> handle_req before it picks up the request looks like another way to
> get there.)
>
> I reproduced it in QEMU with dummy_hcd + raw-gadget emulating an
> RTS5129, widening the window with a debug msleep() right after the
> host->removing check and unplugging the device during it:
>
>   INFO: task kworker/u10:3:65 blocked for more than 20 seconds.
>   Workqueue: kmemstick memstick_check
>    __wait_for_common
>    memstick_check
>
>   INFO: task kworker/1:1:33 blocked for more than 20 seconds.
>   Workqueue: usb_hub_wq hub_event
>    __flush_workqueue
>    memstick_remove_host
>    rtsx_usb_ms_drv_remove
>    platform_remove
>    ...
>    rtsx_usb_disconnect
>    usb_disconnect
>    hub_event
>
> The same test on mainline (500 ms timeout) recovers fine.

Okay, I see. Thanks for looking into this!

So we are solving one problem and re-introducing another. :-)

>
> Since memstick hosts are expected to always complete a request, I think
> the right follow-up is in rtsx_usb_ms: complete requests with an error
> after eject instead of dropping them. I can send that as a separate
> rtsx_usb_ms patch on top of Raj's, if that works for you.

Please do, that would be great!

Kind regards
Uffe

[PATCH] memstick: rtsx_usb_ms: complete requests after eject instead of dropping them

[Nguyen Ngoc Thang]

memstick_check() can pass its host->removing check just before
rtsx_usb_ms_drv_remove() sets eject/removing. Its next request is then
silently dropped: rtsx_usb_ms_request() skips schedule_work() once eject
is set, and drv_remove's cancel_work_sync() can also cancel a queued
handle_req before it picks the request up. Nobody completes
card->mrq_complete.

Once memstick core waits for requests without a timeout ("memstick: core:
wait for request completion before freeing card"), this hangs removal:
memstick_check() never returns and memstick_remove_host() blocks in
flush_workqueue():

  INFO: task kworker/u10:3:65 blocked for more than 20 seconds.
  Workqueue: kmemstick memstick_check
   __wait_for_common
   memstick_check

  INFO: task kworker/1:1:33 blocked for more than 20 seconds.
  Workqueue: usb_hub_wq hub_event
   __flush_workqueue
   memstick_remove_host
   rtsx_usb_ms_drv_remove

Never drop a request. rtsx_usb_ms_request() always schedules handle_req,
and handle_req fails requests with -ENOMEDIUM once eject is set, without
touching the device. drv_remove flushes handle_req instead of cancelling
it, and cancels it only after memstick_remove_host(), when no new
request can arrive, so it cannot run on a freed host.

The host_mutex drain in drv_remove is removed: handle_req always leaves
host->req NULL when it finishes, so the drain never had anything to do,
and it would now race with handle_req.

Fixes: 99451dceeb5f ("memstick: Add realtek USB memstick host driver")
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/all/20260924204142.607-1-rajojha047@gmail.com/
Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
---
This applies on top of Raj's patch:
https://lore.kernel.org/all/20260924204142.607-1-rajojha047@gmail.com/

Tested in QEMU with dummy_hcd + raw-gadget emulating an RTS5129, with a
debug msleep() after the host->removing check in memstick_check() and
the device unplugged during it: with Raj's patch alone removal hangs as
above; with this patch on top it completes, and the original UAF
reproducer stays clean.

 drivers/memstick/host/rtsx_usb_ms.c | 31 ++++++++++-------------------
 1 file changed, 11 insertions(+), 20 deletions(-)

diff --git a/drivers/memstick/host/rtsx_usb_ms.c b/drivers/memstick/host/rtsx_usb_ms.c
index beadc389f15f..d5b3a96fc609 100644
--- a/drivers/memstick/host/rtsx_usb_ms.c
+++ b/drivers/memstick/host/rtsx_usb_ms.c
@@ -27,7 +27,6 @@ struct rtsx_usb_ms {
 	struct memstick_host	*msh;
 	struct memstick_request	*req;
 
-	struct mutex		host_mutex;
 	struct work_struct	handle_req;
 	struct delayed_work	poll_card;
 
@@ -514,6 +513,13 @@ static void rtsx_usb_ms_handle_req(struct work_struct *work)
 	struct memstick_host *msh = host->msh;
 	int rc;
 
+	/* Fail requests after eject so their waiters are released. */
+	if (host->eject) {
+		while (!memstick_next_req(msh, &host->req))
+			host->req->error = -ENOMEDIUM;
+		return;
+	}
+
 	if (!host->req) {
 		pm_runtime_get_sync(ms_dev(host));
 		do {
@@ -547,8 +553,7 @@ static void rtsx_usb_ms_request(struct memstick_host *msh)
 
 	dev_dbg(ms_dev(host), "--> %s\n", __func__);
 
-	if (!host->eject)
-		schedule_work(&host->handle_req);
+	schedule_work(&host->handle_req);
 }
 
 static int rtsx_usb_ms_set_param(struct memstick_host *msh,
@@ -781,7 +786,6 @@ static int rtsx_usb_ms_drv_probe(struct platform_device *pdev)
 	host->power_mode = MEMSTICK_POWER_OFF;
 	platform_set_drvdata(pdev, host);
 
-	mutex_init(&host->host_mutex);
 	INIT_WORK(&host->handle_req, rtsx_usb_ms_handle_req);
 
 	INIT_DELAYED_WORK(&host->poll_card, rtsx_usb_ms_poll_card);
@@ -812,27 +816,12 @@ static void rtsx_usb_ms_drv_remove(struct platform_device *pdev)
 {
 	struct rtsx_usb_ms *host = platform_get_drvdata(pdev);
 	struct memstick_host *msh = host->msh;
-	int err;
 
 	host->eject = true;
 	msh->removing = true;
-	cancel_work_sync(&host->handle_req);
+	flush_work(&host->handle_req);
 	cancel_delayed_work_sync(&host->poll_card);
 
-	mutex_lock(&host->host_mutex);
-	if (host->req) {
-		dev_dbg(ms_dev(host),
-			"%s: Controller removed during transfer\n",
-			dev_name(&msh->dev));
-		host->req->error = -ENOMEDIUM;
-		do {
-			err = memstick_next_req(msh, &host->req);
-			if (!err)
-				host->req->error = -ENOMEDIUM;
-		} while (!err);
-	}
-	mutex_unlock(&host->host_mutex);
-
 	/* Balance possible unbalanced usage count
 	 * e.g. unconditional module removal
 	 */
@@ -841,6 +830,8 @@ static void rtsx_usb_ms_drv_remove(struct platform_device *pdev)
 
 	pm_runtime_disable(ms_dev(host));
 	memstick_remove_host(msh);
+	/* No card, no new requests; wait for the last failed one to finish. */
+	cancel_work_sync(&host->handle_req);
 	dev_dbg(ms_dev(host),
 		": Realtek USB Memstick controller has been removed\n");
 	memstick_free_host(msh);
-- 
2.43.0

Re: [PATCH] memstick: rtsx_usb_ms: complete requests after eject instead of dropping them

[Ulf Hansson]

On Wed, Sep 30, 2026 at 4:07 PM Nguyen Ngoc Thang
<ngocthang2710.1999@gmail.com> wrote:
>
> memstick_check() can pass its host->removing check just before
> rtsx_usb_ms_drv_remove() sets eject/removing. Its next request is then
> silently dropped: rtsx_usb_ms_request() skips schedule_work() once eject
> is set, and drv_remove's cancel_work_sync() can also cancel a queued
> handle_req before it picks the request up. Nobody completes
> card->mrq_complete.
>
> Once memstick core waits for requests without a timeout ("memstick: core:
> wait for request completion before freeing card"), this hangs removal:
> memstick_check() never returns and memstick_remove_host() blocks in
> flush_workqueue():
>
>   INFO: task kworker/u10:3:65 blocked for more than 20 seconds.
>   Workqueue: kmemstick memstick_check
>    __wait_for_common
>    memstick_check
>
>   INFO: task kworker/1:1:33 blocked for more than 20 seconds.
>   Workqueue: usb_hub_wq hub_event
>    __flush_workqueue
>    memstick_remove_host
>    rtsx_usb_ms_drv_remove
>
> Never drop a request. rtsx_usb_ms_request() always schedules handle_req,
> and handle_req fails requests with -ENOMEDIUM once eject is set, without
> touching the device. drv_remove flushes handle_req instead of cancelling
> it, and cancels it only after memstick_remove_host(), when no new
> request can arrive, so it cannot run on a freed host.
>
> The host_mutex drain in drv_remove is removed: handle_req always leaves
> host->req NULL when it finishes, so the drain never had anything to do,
> and it would now race with handle_req.
>
> Fixes: 99451dceeb5f ("memstick: Add realtek USB memstick host driver")
> Cc: stable@vger.kernel.org
> Link: https://lore.kernel.org/all/20260924204142.607-1-rajojha047@gmail.com/
> Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>

Applied for fixes, thanks!

Kind regards
Uffe


> ---
> This applies on top of Raj's patch:
> https://lore.kernel.org/all/20260924204142.607-1-rajojha047@gmail.com/
>
> Tested in QEMU with dummy_hcd + raw-gadget emulating an RTS5129, with a
> debug msleep() after the host->removing check in memstick_check() and
> the device unplugged during it: with Raj's patch alone removal hangs as
> above; with this patch on top it completes, and the original UAF
> reproducer stays clean.
>
>  drivers/memstick/host/rtsx_usb_ms.c | 31 ++++++++++-------------------
>  1 file changed, 11 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/memstick/host/rtsx_usb_ms.c b/drivers/memstick/host/rtsx_usb_ms.c
> index beadc389f15f..d5b3a96fc609 100644
> --- a/drivers/memstick/host/rtsx_usb_ms.c
> +++ b/drivers/memstick/host/rtsx_usb_ms.c
> @@ -27,7 +27,6 @@ struct rtsx_usb_ms {
>         struct memstick_host    *msh;
>         struct memstick_request *req;
>
> -       struct mutex            host_mutex;
>         struct work_struct      handle_req;
>         struct delayed_work     poll_card;
>
> @@ -514,6 +513,13 @@ static void rtsx_usb_ms_handle_req(struct work_struct *work)
>         struct memstick_host *msh = host->msh;
>         int rc;
>
> +       /* Fail requests after eject so their waiters are released. */
> +       if (host->eject) {
> +               while (!memstick_next_req(msh, &host->req))
> +                       host->req->error = -ENOMEDIUM;
> +               return;
> +       }
> +
>         if (!host->req) {
>                 pm_runtime_get_sync(ms_dev(host));
>                 do {
> @@ -547,8 +553,7 @@ static void rtsx_usb_ms_request(struct memstick_host *msh)
>
>         dev_dbg(ms_dev(host), "--> %s\n", __func__);
>
> -       if (!host->eject)
> -               schedule_work(&host->handle_req);
> +       schedule_work(&host->handle_req);
>  }
>
>  static int rtsx_usb_ms_set_param(struct memstick_host *msh,
> @@ -781,7 +786,6 @@ static int rtsx_usb_ms_drv_probe(struct platform_device *pdev)
>         host->power_mode = MEMSTICK_POWER_OFF;
>         platform_set_drvdata(pdev, host);
>
> -       mutex_init(&host->host_mutex);
>         INIT_WORK(&host->handle_req, rtsx_usb_ms_handle_req);
>
>         INIT_DELAYED_WORK(&host->poll_card, rtsx_usb_ms_poll_card);
> @@ -812,27 +816,12 @@ static void rtsx_usb_ms_drv_remove(struct platform_device *pdev)
>  {
>         struct rtsx_usb_ms *host = platform_get_drvdata(pdev);
>         struct memstick_host *msh = host->msh;
> -       int err;
>
>         host->eject = true;
>         msh->removing = true;
> -       cancel_work_sync(&host->handle_req);
> +       flush_work(&host->handle_req);
>         cancel_delayed_work_sync(&host->poll_card);
>
> -       mutex_lock(&host->host_mutex);
> -       if (host->req) {
> -               dev_dbg(ms_dev(host),
> -                       "%s: Controller removed during transfer\n",
> -                       dev_name(&msh->dev));
> -               host->req->error = -ENOMEDIUM;
> -               do {
> -                       err = memstick_next_req(msh, &host->req);
> -                       if (!err)
> -                               host->req->error = -ENOMEDIUM;
> -               } while (!err);
> -       }
> -       mutex_unlock(&host->host_mutex);
> -
>         /* Balance possible unbalanced usage count
>          * e.g. unconditional module removal
>          */
> @@ -841,6 +830,8 @@ static void rtsx_usb_ms_drv_remove(struct platform_device *pdev)
>
>         pm_runtime_disable(ms_dev(host));
>         memstick_remove_host(msh);
> +       /* No card, no new requests; wait for the last failed one to finish. */
> +       cancel_work_sync(&host->handle_req);
>         dev_dbg(ms_dev(host),
>                 ": Realtek USB Memstick controller has been removed\n");
>         memstick_free_host(msh);
> --
> 2.43.0
>