[PATCH] USB: serial: garmin_gps: fix deadlock on reset request

[PATCH] USB: serial: garmin_gps: fix deadlock on reset request

[Johan Hovold]

The garmin_gps driver has a reset feature that is supposed to reset the
device when writing a private reset request packet. The reset is however
done synchronously from the write() callback (and without holding the
required device lock) which leads to a deadlock in the line discipline
when hanging up the port as part of disconnect.

Switch to using the asynchronous reset interface while dropping the
redundant and synchronous stopping of the interrupt urb (write()
callbacks are supposed to be atomic and USB core, USB serial core and
the driver itself will unlink and kill all URBs during disconnect).

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: Josef Bacik <josef@toxicpanda.com>
Link: https://lore.kernel.org/2026102733-prayer-drainer-56f3@gregkh
Cc: stable@vger.kernel.org
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/garmin_gps.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/serial/garmin_gps.c b/drivers/usb/serial/garmin_gps.c
index abfc03e44ca9..f1e69eca2601 100644
--- a/drivers/usb/serial/garmin_gps.c
+++ b/drivers/usb/serial/garmin_gps.c
@@ -823,10 +823,9 @@ static void priv_status_resp(struct usb_serial_port *port)
  * Garmin specific driver functions
  ******************************************************************************/
 
-static int process_resetdev_request(struct usb_serial_port *port)
+static void process_resetdev_request(struct usb_serial_port *port)
 {
 	unsigned long flags;
-	int status;
 	struct garmin_data *garmin_data_p = usb_get_serial_port_data(port);
 
 	spin_lock_irqsave(&garmin_data_p->lock, flags);
@@ -835,13 +834,8 @@ static int process_resetdev_request(struct usb_serial_port *port)
 	garmin_data_p->serial_num = 0;
 	spin_unlock_irqrestore(&garmin_data_p->lock, flags);
 
-	usb_kill_urb(port->interrupt_in_urb);
-	dev_dbg(&port->dev, "%s - usb_reset_device\n", __func__);
-	status = usb_reset_device(port->serial->dev);
-	if (status)
-		dev_dbg(&port->dev, "%s - usb_reset_device failed: %d\n",
-			__func__, status);
-	return status;
+	dev_dbg(&port->dev, "%s - usb_queue_reset_device\n", __func__);
+	usb_queue_reset_device(port->serial->interface);
 }
 
 
-- 
2.55.0

Re: [PATCH] USB: serial: garmin_gps: fix deadlock on reset request

[sashiko-bot]

> The garmin_gps driver has a reset feature that is supposed to reset the
> device when writing a private reset request packet. The reset is however
> done synchronously from the write() callback (and without holding the
> required device lock) which leads to a deadlock in the line discipline
> when hanging up the port as part of disconnect.
> 
> Switch to using the asynchronous reset interface while dropping the
> redundant and synchronous stopping of the interrupt urb (write()
> callbacks are supposed to be atomic and USB core, USB serial core and
> the driver itself will unlink and kill all URBs during disconnect).
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Reported-by: Josef Bacik <josef@toxicpanda.com>
> Link: https://lore.kernel.org/2026102733-prayer-drainer-56f3@gregkh
> Cc: stable@vger.kernel.org
> Signed-off-by: Johan Hovold <johan@kernel.org>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261029101828.865915-1-johan@kernel.org?part=1

Re: [PATCH] USB: serial: garmin_gps: fix deadlock on reset request

[Greg Kroah-Hartman]

On Thu, Oct 29, 2026 at 11:18:28AM +0100, Johan Hovold wrote:
> The garmin_gps driver has a reset feature that is supposed to reset the
> device when writing a private reset request packet. The reset is however
> done synchronously from the write() callback (and without holding the
> required device lock) which leads to a deadlock in the line discipline
> when hanging up the port as part of disconnect.
> 
> Switch to using the asynchronous reset interface while dropping the
> redundant and synchronous stopping of the interrupt urb (write()
> callbacks are supposed to be atomic and USB core, USB serial core and
> the driver itself will unlink and kill all URBs during disconnect).
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Reported-by: Josef Bacik <josef@toxicpanda.com>
> Link: https://lore.kernel.org/2026102733-prayer-drainer-56f3@gregkh
> Cc: stable@vger.kernel.org
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>  drivers/usb/serial/garmin_gps.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/serial/garmin_gps.c b/drivers/usb/serial/garmin_gps.c
> index abfc03e44ca9..f1e69eca2601 100644
> --- a/drivers/usb/serial/garmin_gps.c
> +++ b/drivers/usb/serial/garmin_gps.c
> @@ -823,10 +823,9 @@ static void priv_status_resp(struct usb_serial_port *port)
>   * Garmin specific driver functions
>   ******************************************************************************/
>  
> -static int process_resetdev_request(struct usb_serial_port *port)
> +static void process_resetdev_request(struct usb_serial_port *port)
>  {
>  	unsigned long flags;
> -	int status;
>  	struct garmin_data *garmin_data_p = usb_get_serial_port_data(port);
>  
>  	spin_lock_irqsave(&garmin_data_p->lock, flags);
> @@ -835,13 +834,8 @@ static int process_resetdev_request(struct usb_serial_port *port)
>  	garmin_data_p->serial_num = 0;
>  	spin_unlock_irqrestore(&garmin_data_p->lock, flags);
>  
> -	usb_kill_urb(port->interrupt_in_urb);
> -	dev_dbg(&port->dev, "%s - usb_reset_device\n", __func__);
> -	status = usb_reset_device(port->serial->dev);
> -	if (status)
> -		dev_dbg(&port->dev, "%s - usb_reset_device failed: %d\n",
> -			__func__, status);
> -	return status;
> +	dev_dbg(&port->dev, "%s - usb_queue_reset_device\n", __func__);
> +	usb_queue_reset_device(port->serial->interface);
>  }
>  
>  
> -- 
> 2.55.0
>

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>