Skip to contentSkip to conversation

Patch "xhci: Fix race related to abort operation" has been added to the 4.9-stable tree

PATCH [email protected] 1 participant 1 message Jan 9, 2017

Subsystems (3)
lore
Share & tools
lore

[email protected]

author maintainer Applied
#0 lore raw

This is a note to let you know that I've just added the patch titled

xhci: Fix race related to abort operation

to the 4.9-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
xhci-fix-race-related-to-abort-operation.patch
and it can be found in the queue-4.9 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 1 line

From 1c111b6c3844a142e03bcfc2fa17bfbdea08e9dc Mon Sep 17 00:00:00 2001

From: OGAWA Hirofumi <[email protected]>
Date: Tue, 3 Jan 2017 18:28:51 +0200
Subject: xhci: Fix race related to abort operation

From: OGAWA Hirofumi <[email protected]>

commit 1c111b6c3844a142e03bcfc2fa17bfbdea08e9dc upstream.

Current abort operation has race.

    xhci_handle_command_timeout()
      xhci_abort_cmd_ring()
        xhci_write_64(CMD_RING_ABORT)
        xhci_handshake(5s)
	  do {
	    check CMD_RING_RUNNING
            udelay(1)
					 ...
					 COMP_CMD_ABORT event
					 COMP_CMD_STOP event
					 xhci_handle_stopped_cmd_ring()
					   restart cmd_ring
                                           CMD_RING_RUNNING become 1 again
	  } while ()
          return -ETIMEDOUT
        xhci_write_64(CMD_RING_ABORT)
        /* can abort random command */

To do abort operation correctly, we have to wait both of COMP_CMD_STOP event and negation of CMD_RING_RUNNING.

But like above, while timeout handler is waiting negation of CMD_RING_RUNNING, event handler can restart cmd_ring. So timeout handler never be notice negation of CMD_RING_RUNNING, and retry of CMD_RING_ABORT can abort random command (BTW, I guess retry of CMD_RING_ABORT was workaround of this race).

To fix this race, this moves xhci_handle_stopped_cmd_ring() to xhci_abort_cmd_ring(). And timeout handler waits COMP_CMD_STOP event.

At this point, timeout handler is owner of cmd_ring, and safely restart cmd_ring by using xhci_handle_stopped_cmd_ring().

[FWIW, as bonus, this way would be easily extend to add CMD_RING_PAUSE operation]

[locks edited as patch is rebased on other locking fixes -Mathias]
---
 drivers/usb/host/xhci-mem.c  |    1 
 drivers/usb/host/xhci-ring.c |  169 ++++++++++++++++++++++---------------------
 drivers/usb/host/xhci.h      |    1 
 3 files changed, 91 insertions(+), 80 deletions(-)
drivers/usb/host/xhci-mem.c +1 -0
@@ -2379,6 +2379,7 @@ int xhci_mem_init(struct xhci_hcd *xhci,
 
 	/* init command timeout work */
 	INIT_DELAYED_WORK(&xhci->cmd_timer, xhci_handle_command_timeout);
+	init_completion(&xhci->cmd_ring_stop_completion);
 
 	page_size = readl(&xhci->op_regs->page_size);
 	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
drivers/usb/host/xhci-ring.c +89 -80

226 lines not on this page · raw message

drivers/usb/host/xhci.h +1 -0
@@ -1572,6 +1572,7 @@ struct xhci_hcd {
 	struct list_head        cmd_list;
 	unsigned int		cmd_ring_reserved_trbs;
 	struct delayed_work	cmd_timer;
+	struct completion	cmd_ring_stop_completion;
 	struct xhci_command	*current_cmd;
 	struct xhci_ring	*event_ring;
 	struct xhci_erst	erst;

Patches currently in stable-queue which might be from [email protected] are

queue-4.9/xhci-use-delayed_work-instead-of-timer-for-command-timeout.patch queue-4.9/xhci-fix-race-related-to-abort-operation.patch