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@xxxxxxxxx> 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@xxxxxxxxxxxxxxx
> Link: https://lore.kernel.org/all/20260924204142.607-1-rajojha047@xxxxxxxxx/
> Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@xxxxxxxxx>

Applied for fixes, thanks!

Kind regards
Uffe


> ---
> This applies on top of Raj's patch:
> https://lore.kernel.org/all/20260924204142.607-1-rajojha047@xxxxxxxxx/
>
> 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
>