mmc: vub300: defer reset until cmd_mutex is unlocked

vub300_cmndwork_thread() holds cmd_mutex while it sends a command and
waits for the command response.  If the response wait times out,
__vub300_command_response() kills the command URBs and then synchronously
resets the USB device through usb_reset_device().

That reset path re-enters the driver through vub300_pre_reset(), which
also takes cmd_mutex.  The worker therefore tries to acquire the same
mutex recursively while it is still holding it from the command path.

This issue was found by our static analysis tool and then manually
reviewed against the current tree.

The grounded PoC kept the real worker and timeout/reset carrier:

  vub300_cmndwork_thread()
  __vub300_command_response()
  usb_lock_device_for_reset()
  usb_reset_device()
  vub300_pre_reset()

Lockdep reported the same-task recursive acquisition on cmd_mutex:

  WARNING: possible recursive locking detected
  ... (&test_vub300.cmd_mutex) ... at: usb_reset_device... [vuln_msv]
  ... (&test_vub300.cmd_mutex) ... at: vub300_cmndwork_thread+0x12/0x20 [vuln_msv]
  Workqueue: vub300_cmd_wq vub300_cmndwork_thread [vuln_msv]
  *** DEADLOCK ***

Return a flag from __vub300_command_response() when the timeout path needs
a device reset, then perform the reset after vub300_cmndwork_thread() has
cleared the in-flight command state and dropped cmd_mutex.  The reset is
still attempted before mmc_request_done(), preserving the existing request
completion ordering while avoiding the recursive lock.

Fixes: 88095e7b473a ("mmc: Add new VUB300 USB-to-SD/SDIO/MMC driver")
Cc: stable@vger.kernel.org
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
Signed-off-by: Ulf Hansson <ulfh@kernel.org>

diff --git a/drivers/mmc/host/vub300.c b/drivers/mmc/host/vub300.c
index c1c21e95f5bf83d006698e0de6f71a0bbd5ea218..2dae474dcd067993aef16669fe9138821a551492 100644 (file)
--- a/drivers/mmc/host/vub300.c
+++ b/drivers/mmc/host/vub300.c
@@ -1586,7 +1586,7 @@ static int __command_write_data(struct vub300_mmc_host *vub300,
        return linear_length;
 }
 
-static void __vub300_command_response(struct vub300_mmc_host *vub300,
+static bool __vub300_command_response(struct vub300_mmc_host *vub300,
                                      struct mmc_command *cmd,
                                      struct mmc_data *data, int data_length)
 {
@@ -1598,17 +1598,11 @@ static void __vub300_command_response(struct vub300_mmc_host *vub300,
                                            msecs_to_jiffies(msec_timeout));
        if (respretval == 0) { /* TIMED OUT */
                /* we don't know which of "out" and "res" if any failed */
-               int result;
                vub300->usb_timed_out = 1;
                usb_kill_urb(vub300->command_out_urb);
                usb_kill_urb(vub300->command_res_urb);
                cmd->error = -ETIMEDOUT;
-               result = usb_lock_device_for_reset(vub300->udev,
-                                                  vub300->interface);
-               if (result == 0) {
-                       result = usb_reset_device(vub300->udev);
-                       usb_unlock_device(vub300->udev);
-               }
+               return true;
        } else if (respretval < 0) {
                /* we don't know which of "out" and "res" if any failed */
                usb_kill_urb(vub300->command_out_urb);
@@ -1704,6 +1698,8 @@ static void __vub300_command_response(struct vub300_mmc_host *vub300,
        } else {
                cmd->error = -EINVAL;
        }
+
+       return false;
 }
 
 static void construct_request_response(struct vub300_mmc_host *vub300,
@@ -1749,6 +1745,7 @@ static void vub300_cmndwork_thread(struct work_struct *work)
                struct mmc_request *req = vub300->req;
                struct mmc_command *cmd = vub300->cmd;
                struct mmc_data *data = vub300->data;
+               bool reset_device;
                int data_length;
                mutex_lock(&vub300->cmd_mutex);
                init_completion(&vub300->command_complete);
@@ -1771,7 +1768,8 @@ static void vub300_cmndwork_thread(struct work_struct *work)
                        data_length = __command_read_data(vub300, cmd, data);
                else
                        data_length = __command_write_data(vub300, cmd, data);
-               __vub300_command_response(vub300, cmd, data, data_length);
+               reset_device = __vub300_command_response(vub300, cmd,
+                                                        data, data_length);
                vub300->req = NULL;
                vub300->cmd = NULL;
                vub300->data = NULL;
@@ -1779,6 +1777,16 @@ static void vub300_cmndwork_thread(struct work_struct *work)
                        if (cmd->error == -ENOMEDIUM)
                                check_vub300_port_status(vub300);
                        mutex_unlock(&vub300->cmd_mutex);
+                       if (reset_device) {
+                               int result;
+
+                               result = usb_lock_device_for_reset(vub300->udev,
+                                                                  vub300->interface);
+                               if (result == 0) {
+                                       result = usb_reset_device(vub300->udev);
+                                       usb_unlock_device(vub300->udev);
+                               }
+                       }
                        mmc_request_done(vub300->mmc, req);
                        kref_put(&vub300->kref, vub300_delete);
                        return;