Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 1f4cb010 authored by Chris Bainbridge's avatar Chris Bainbridge Committed by Jack Pham
Browse files

usb: host: xhci: add mutex for non-thread-safe data 

Regression in commit 638139eb ("usb: hub: allow to process more usb
hub events in parallel")

The regression resulted in intermittent failure to initialise a 10-port
hub (with three internal VL812 4-port hub controllers) on boot, with a
failure rate of around 8%, due to multiple race conditions when
accessing addr_dev and slot_id in struct xhci_hcd.

This regression also exposed a problem with xhci_setup_device, which
"should be protected by the usb_address0_mutex" but no longer is due to

commit 6fecd4f2 ("USB: separate usb_address0 mutexes for each bus")

With separate buses (and locks) it is no longer the case that a single
lock will protect xhci_setup_device from accesses by two parallel
threads processing events on the two buses.

Fix this by adding a mutex to protect addr_dev and slot_id in struct
xhci_hcd, and by making the assignment of slot_id atomic.

Fixes multiple boot errors:

[ 0.583008] xhci_hcd 0000:00:14.0: Bad Slot ID 2
[ 0.583009] xhci_hcd 0000:00:14.0: Could not allocate xHCI USB device data structures
[ 0.583012] usb usb1-port3: couldn't allocate usb_device

And:

[ 0.637409] xhci_hcd 0000:00:14.0: Error while assigning device slot ID
[ 0.637417] xhci_hcd 0000:00:14.0: Max number of devices this xHCI host supports is 32.
[ 0.637421] usb usb1-port1: couldn't allocate usb_device

And:

[ 0.753372] xhci_hcd 0000:00:14.0: ERROR: unexpected setup context command completion code 0x0.
[ 0.753373] usb 1-3: hub failed to enable device, error -22
[ 0.753400] xhci_hcd 0000:00:14.0: Error while assigning device slot ID
[ 0.753402] xhci_hcd 0000:00:14.0: Max number of devices this xHCI host supports is 32.
[ 0.753403] usb usb1-port3: couldn't allocate usb_device

And:

[ 11.018386] usb 1-3: device descriptor read/all, error -110

And:

[ 5.753838] xhci_hcd 0000:00:14.0: Timeout while waiting for setup device command

Tested with 200 reboots, resulting in no USB hub init related errors.

Fixes: 638139eb ("usb: hub: allow to process more usb hub events in parallel")
Link: https://lkml.kernel.org/g/CAP-bSRb=A0iEYobdGCLpwynS7pkxpt_9ZnwyZTPVAoy0Y=Zo3Q@mail.gmail.com


Signed-off-by: default avatarChris Bainbridge <chris.bainbridge@gmail.com>
Cc: <stable@vger.kernel.org> # 3.18+
[changed git commit description style for checkpatch -Mathias]
Signed-off-by: default avatarMathias Nyman <mathias.nyman@linux.intel.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Change-Id: I288ba751586d302005f0214157a98afabcd6e610
Git-commit: a00918d0521df1c7a2ec9143142a3ea998c8526d
Git-repo: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git


Signed-off-by: default avatarJack Pham <jackp@codeaurora.org>
parent 45b16e7b

drivers/usb/host/xhci.c

+30 −19
Original line number Diff line number Diff line
@@ -3695,18 +3695,21 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev) 
{

	struct xhci_hcd *xhci = hcd_to_xhci(hcd);

	unsigned long flags;

	int ret;

	int ret, slot_id;

	struct xhci_command *command;



	command = xhci_alloc_command(xhci, false, false, GFP_KERNEL);

	if (!command)

		return 0;



	/* xhci->slot_id and xhci->addr_dev are not thread-safe */

	mutex_lock(&xhci->mutex);

	spin_lock_irqsave(&xhci->lock, flags);

	command->completion = &xhci->addr_dev;

	ret = xhci_queue_slot_control(xhci, command, TRB_ENABLE_SLOT, 0);

	if (ret) {

		spin_unlock_irqrestore(&xhci->lock, flags);

		mutex_unlock(&xhci->mutex);

		xhci_dbg(xhci, "FIXME: allocate a command ring segment\n");

		kfree(command);

		return 0;
@@ -3715,8 +3718,10 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev) 
	spin_unlock_irqrestore(&xhci->lock, flags);



	wait_for_completion(command->completion);

	slot_id = xhci->slot_id;

	mutex_unlock(&xhci->mutex);



	if (!xhci->slot_id || command->status != COMP_SUCCESS) {

	if (!slot_id || command->status != COMP_SUCCESS) {

		xhci_err(xhci, "Error while assigning device slot ID\n");

		xhci_err(xhci, "Max number of devices this xHCI host supports is %u.\n",

				HCS_MAX_SLOTS(
@@ -3741,11 +3746,11 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev) 
	 * xhci_discover_or_reset_device(), which may be called as part of

	 * mass storage driver error handling.

	 */

	if (!xhci_alloc_virt_device(xhci, xhci->slot_id, udev, GFP_NOIO)) {

	if (!xhci_alloc_virt_device(xhci, slot_id, udev, GFP_NOIO)) {

		xhci_warn(xhci, "Could not allocate xHCI USB device data structures\n");

		goto disable_slot;

	}

	udev->slot_id = xhci->slot_id;

	udev->slot_id = slot_id;



#ifndef CONFIG_USB_DEFAULT_PERSIST

	/*
@@ -3791,12 +3796,15 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev, 
	struct xhci_slot_ctx *slot_ctx;

	struct xhci_input_control_ctx *ctrl_ctx;

	u64 temp_64;

	struct xhci_command *command;

	struct xhci_command *command = NULL;



	mutex_lock(&xhci->mutex);



	if (!udev->slot_id) {

		xhci_dbg_trace(xhci, trace_xhci_dbg_address,

				"Bad Slot ID %d", udev->slot_id);

		return -EINVAL;

		ret = -EINVAL;

		goto out;

	}



	virt_dev = xhci->devs[udev->slot_id];
@@ -3809,7 +3817,8 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev, 
		 */

		xhci_warn(xhci, "Virt dev invalid for slot_id 0x%x!\n",

			udev->slot_id);

		return -EINVAL;

		ret = -EINVAL;

		goto out;

	}



	if (setup == SETUP_CONTEXT_ONLY) {
@@ -3817,13 +3826,15 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev, 
		if (GET_SLOT_STATE(le32_to_cpu(slot_ctx->dev_state)) ==

		    SLOT_STATE_DEFAULT) {

			xhci_dbg(xhci, "Slot already in default state\n");

			return 0;

			goto out;

		}

	}



	command = xhci_alloc_command(xhci, false, false, GFP_KERNEL);

	if (!command)

		return -ENOMEM;

	if (!command) {

		ret = -ENOMEM;

		goto out;

	}



	command->in_ctx = virt_dev->in_ctx;

	command->completion = &xhci->addr_dev;
@@ -3833,8 +3844,8 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev, 
	if (!ctrl_ctx) {

		xhci_warn(xhci, "%s: Could not get input context, bad type.\n",

				__func__);

		kfree(command);

		return -EINVAL;

		ret = -EINVAL;

		goto out;

	}

	/*

	 * If this is the first Set Address since device plug-in or
@@ -3861,8 +3872,7 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev, 
		spin_unlock_irqrestore(&xhci->lock, flags);

		xhci_dbg_trace(xhci, trace_xhci_dbg_address,

				"FIXME: allocate a command ring segment");

		kfree(command);

		return ret;

		goto out;

	}

	xhci_ring_cmd_db(xhci);

	spin_unlock_irqrestore(&xhci->lock, flags);
@@ -3909,10 +3919,8 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev, 
		ret = -EINVAL;

		break;

	}

	if (ret) {

		kfree(command);

		return ret;

	}

	if (ret)

		goto out;

	temp_64 = xhci_read_64(xhci, &xhci->op_regs->dcbaa_ptr);

	xhci_dbg_trace(xhci, trace_xhci_dbg_address,

			"Op regs DCBAA ptr = %#016llx", temp_64);
@@ -3945,8 +3953,10 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev, 
	xhci_dbg_trace(xhci, trace_xhci_dbg_address,

		       "Internal device address = %d",

		       le32_to_cpu(slot_ctx->dev_state) & DEV_ADDR_MASK);

out:

	mutex_unlock(&xhci->mutex);

	kfree(command);

	return 0;

	return ret;

}



int xhci_address_device(struct usb_hcd *hcd, struct usb_device *udev)
@@ -4872,6 +4882,7 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks) 
		return 0;

	}



	mutex_init(&xhci->mutex);

	xhci->cap_regs = hcd->regs;

	xhci->op_regs = hcd->regs +

		HC_LENGTH(readl(&xhci->cap_regs->hc_capbase));

drivers/usb/host/xhci.h

+2 −0
Original line number Diff line number Diff line
@@ -1497,6 +1497,8 @@ struct xhci_hcd { 
	struct list_head	lpm_failed_devs;



	/* slot enabling and address device helpers */

	/* these are not thread safe so use mutex */

	struct mutex mutex;

	struct completion	addr_dev;

	int slot_id;

	/* For USB 3.0 LPM enable/disable. */