From patchwork Fri Feb 21 01:18:55 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michal Pecio X-Patchwork-Id: 13984716 Received: from mail-ed1-f52.google.com (mail-ed1-f52.google.com [209.85.208.52]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9331B18A6B0; Fri, 21 Feb 2025 01:19:01 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.208.52 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740100743; cv=none; b=jIPW9bfGPkYciXnNjXMxUEJPYEr43HPVUqwuGJlbOm3GLQfONDiOLUoX8WkrlGD1BUzeiPeATZfxurJc1O6trP8+bsQ87/IMb9QHJkzRUg85kOIq/siZu7KTHZ1OaGBF49p+amBLlFBZhTbt3/0p3dxA0//h00GBKc7S5/t+SCo= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740100743; c=relaxed/simple; bh=ZLi32eHzVBzb0+S2Hyi563N7biVmoJrvni7xDdRZ/2E=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=fwXpMrJPQYn2Gq1fgT+QHHZXTcUj1NNrDnYnLk5Mv4UtQD1ZhXzwzb1Dfd4blY2UwcJ1fmEg1HkyWmf/Gss5BjB/CKQEj/8id4hq6yRW2hkkcrHX7XKTc0A8oCqhvxo113RfS+hxDF3PAV1PNRW451gAvxozLNW62SPS76C9QTo= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=KxRAiP5O; arc=none smtp.client-ip=209.85.208.52 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="KxRAiP5O" Received: by mail-ed1-f52.google.com with SMTP id 4fb4d7f45d1cf-5e0939c6456so2687881a12.3; Thu, 20 Feb 2025 17:19:01 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1740100740; x=1740705540; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=0+iEG7L5f/jy2xV7Yyu1dhl1MhKYSR3nFi6wl+Gwc4I=; b=KxRAiP5OwrCFR7pCAYdk/X3bsypJGZQy+sfEzjz125JmS0/e2eY2HRbPRQlgb/k8Mw eGQPwLna72RinBfvKCs7mP/MKWvxMrzY8AXxMkXou2fq+Pj27Hg6cN+PhrLu1bM4cZLF F/a2H3vZCjogFVBVGXX67k7hxSIw/x7ynbut+YiXUh/TPClNy5etHE+mXEcx3UiXBJ3N PjBAb+/WZo3C3H5r5l8X8Awsas3YxWMheamJtfnZdF/ilynRgvqO+ZPe+hlrsbvLISz8 F/bccv1oeV/tj0hWTXn7Gs2dRaLgo+xsJUxEE9nkUR2nSJOR8E9kTo5Yvi1C9NIo1K/K G0GQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1740100740; x=1740705540; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=0+iEG7L5f/jy2xV7Yyu1dhl1MhKYSR3nFi6wl+Gwc4I=; b=XAvBzSd8SvyI1CcL/HR+g5jSz/PClbFSwY0r1UmVJplBOcq8XnxrmIa2A0ZHT5Z3Ip D8JiVC5/bhAnNjOYEKMp7gA015J9Lgdn7/XOAX0UrseS45mHvo0vYPzy81ZGLbEKJZyj O6Jx/9PBl2VaqylLHFSeQxFcQSSdeKoXNCtzpLbl2WkavrUIk66zHLVs0iPPRi2uG2uz 5pJBPTMUb2hdSF4OutgPoY3EdyQu+LqnGcz9RVdlxrdTWpCpjseCMNtSIipEO5uuYKbl agD4RF7VW0Pg037JMxyIryJnlpJ0qM7jHy10m0s+AHLTcNs6mEbrhOiykHhSjb4mdfe3 Hnag== X-Forwarded-Encrypted: i=1; AJvYcCWnp9Gd/ZPiInHNw7XpggZT6kmx7wUgJ9xle2gKuRMi1D5nGz6djevZdK9jQ81NiGYS/9oKIeIvUG3muZc=@vger.kernel.org, AJvYcCXVWAZyT3Um6nnvmNu3ZVIz6FbX9YeyTr743SfPMwdJnF38zU8t3Xu201GyDwlrZVCF6OJT6xBg/LSe@vger.kernel.org X-Gm-Message-State: AOJu0YxR01eWKpy5hnhmo75HfSmSiT4rIGNyb4FfMeK7uZ28wnd/hTlg o+CyP/8NhIwJfYyzh7yw12OZ/tlEMkbaYTyzYbqsfGphTB5GgAdq X-Gm-Gg: ASbGncu7RI6vUkATuh3jqCSLcQOO08SE/50weFtmEdn+SN5cDESw0InXebFZToq6Lry 1eDAmnCYpcPfL3Fe1UqjSuh3JMDdU43SxT/baJriImV2WJ2bAxLNMGQzP0+Sx8YCno4jE+nTwbw 60J1JZO+tjJsnXt7IWAxWJar2kAkW5Rn3PFd7/ETmNu3FEKCVdodtaYXhOGyHkX9/1IWol/SSoA QqHc5XZD0BDUk119oUH7AZppX6JlBXYORsaJVQE0CZVLwXo8c/kiXM4TZm9eGMY+sW2CxiCC8Rq gby2mK4KSieqnGPG03EAuZPUqQblaiUW X-Google-Smtp-Source: AGHT+IFMEv+hXN3BMjMZTmOB8KI/HxEtm63PLVWUj3GnEV/M7NqMcgdBcB/JZ6vNL3QoQNNJ5LiPXw== X-Received: by 2002:a17:907:7eaa:b0:ab7:c00c:680f with SMTP id a640c23a62f3a-abc0de4f6f8mr79472366b.53.1740100739565; Thu, 20 Feb 2025 17:18:59 -0800 (PST) Received: from foxbook (adtt173.neoplus.adsl.tpnet.pl. [79.185.231.173]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-abbb860fc11sm624441066b.124.2025.02.20.17.18.58 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Thu, 20 Feb 2025 17:18:59 -0800 (PST) Date: Fri, 21 Feb 2025 02:18:55 +0100 From: Michal Pecio To: Mathias Nyman Cc: Mathias Nyman , Greg Kroah-Hartman , Niklas Neronin , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v2 3/5] usb: xhci: Fix isochronous Ring Underrun/Overrun event handling Message-ID: <20250221021855.0f188018@foxbook> In-Reply-To: <20250221021712.48c07fe0@foxbook> References: <20250210083718.2dd337c3@foxbook> <20250210084220.3e5414e9@foxbook> <7bb25848-c80e-4ba8-8790-8628951806b3@linux.intel.com> <20250221021712.48c07fe0@foxbook> Precedence: bulk X-Mailing-List: linux-usb@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 The TRB pointer of these events points at enqueue at the time of error occurrence on xHCI 1.1+ HCs or it's NULL on older ones. By the time we are handling the event, a new TD may be queued at this ring position. I can trigger this race by rising interrupt moderation to increase IRQ handling delay. Similar delay may occur naturally due to system load. If this ever happens after a Missed Service Error, missed TDs will be skipped and the new TD processed as if it matched the event. It could be given back prematurely, risking data loss or buffer UAF by the xHC. Don't complete TDs on xrun events and don't warn if queued TDs don't match the event's TRB pointer, which can be NULL or a link/no-op TRB. Now that it's safe, also handle xrun events if the skip flag is clear. This ensures completion of any TD stuck in 'error mid TD' state right before the xrun event, which could happen if a driver submits a finite number of URBs to a buggy HC and then an error occurs on the last TD. Signed-off-by: Michal Pecio --- drivers/usb/host/xhci-ring.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index a278f284f4c0..4cf17801a233 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -2632,6 +2632,7 @@ static int handle_tx_event(struct xhci_hcd *xhci, int status = -EINPROGRESS; struct xhci_ep_ctx *ep_ctx; u32 trb_comp_code; + bool ring_xrun_event = false; slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event->flags)); ep_index = TRB_TO_EP_ID(le32_to_cpu(event->flags)) - 1; @@ -2738,14 +2739,12 @@ static int handle_tx_event(struct xhci_hcd *xhci, * Underrun Event for OUT Isoch endpoint. */ xhci_dbg(xhci, "Underrun event on slot %u ep %u\n", slot_id, ep_index); - if (ep->skip) - break; - return 0; + ring_xrun_event = true; + break; case COMP_RING_OVERRUN: xhci_dbg(xhci, "Overrun event on slot %u ep %u\n", slot_id, ep_index); - if (ep->skip) - break; - return 0; + ring_xrun_event = true; + break; case COMP_MISSED_SERVICE_ERROR: /* * When encounter missed service error, one or more isoc tds @@ -2818,6 +2817,7 @@ static int handle_tx_event(struct xhci_hcd *xhci, */ if (trb_comp_code != COMP_STOPPED && trb_comp_code != COMP_STOPPED_LENGTH_INVALID && + !ring_xrun_event && !ep_ring->last_td_was_short) { xhci_warn(xhci, "Event TRB for slot %u ep %u with no TDs queued\n", slot_id, ep_index); @@ -2862,6 +2862,9 @@ static int handle_tx_event(struct xhci_hcd *xhci, */ } while (ep->skip); + if (ring_xrun_event) + return 0; /* don't warn or complete any TDs */ + if (!ep_seg) { /* * Skip the Force Stopped Event. The 'ep_trb' of FSE is not in the current From patchwork Fri Feb 21 01:20:26 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michal Pecio X-Patchwork-Id: 13984717 Received: from mail-ej1-f46.google.com (mail-ej1-f46.google.com [209.85.218.46]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id CB96438F80; Fri, 21 Feb 2025 01:20:31 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.218.46 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740100834; cv=none; b=jZWyVAbSxlGkNq2/cYsWRaxUOL+oqgg0I1j1r+WX7fLNYJPgzVEPtUMiiNNSvw6qqI6/FDehi9EBKOAXbcOWyj7qqs5dy50VBR9ii0bpF63ofzGA27X8ZXUQe8eh0FZDEQLsihZ4VPUVPsMxi/hcfxHUP8xa+P82t3VMghuVOxk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740100834; c=relaxed/simple; bh=2JP0fOOE6QK8ybzMtqOP6sxOWXhR9cgy9bGRHa9Rh+4=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=IHNSE0DpNsd+MTW9WGqE5a0DAhB5ppKp5zoxhrWxPH8zZb+zVnLvOQZTfKv+25Yv1jWQ9QANxFVq/9yrkGj8QWmioGQQpUJNm6sYyMRAJHbogouzON7gkV2765b3IkddBespEDj3OZXPJg3eUbg0LDXkqp+G9qt5azgVE1zD6bY= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=adZDcVlX; arc=none smtp.client-ip=209.85.218.46 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="adZDcVlX" Received: by mail-ej1-f46.google.com with SMTP id a640c23a62f3a-aaec61d0f65so330289866b.1; Thu, 20 Feb 2025 17:20:31 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1740100830; x=1740705630; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=zBz1Nat0cY2w8r0Wbkva5eD9bF4XcQmPPZZcToCpqs8=; b=adZDcVlXAkkaWhMk8pw4nBybTDCyJ579jKPTxFiebg7qBJQDQcogtTF0wdRMwtGiAF EqU6yN/e+LfrUOAisH1dqWppIAh5C7hkMfUD6JZx03ok0K3DQ6LDg49LiWfJGEVN2M5h tozmmtwzUl/iDQHzKzK+KwI6fPV+OIDKgfbB9VTkaaJrwR+4A2AQjb8fJzpWd0wSihsj NzMClpK8wg7E0UzKKc5YYZvRuKDxpTgwoftaupauft1vWk2sB4Na1gaulx+bAkJIXM/U UU1jNMg6z62cZ7Eh3fDoj/+ZQZsmqwXoj0An/X4Nxbk2G5/H9XgucvZt1jnTartSyXNJ a+zQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1740100830; x=1740705630; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=zBz1Nat0cY2w8r0Wbkva5eD9bF4XcQmPPZZcToCpqs8=; b=jMdbNuhcMyb5D6B10L2F3H1ohRp4f3NPgoMly7DANmzxOXSuSfxCYA+E3UjHiBIeU0 lDCKCN51hkgzPsFJIZh9XS01+m3YOK+FVnqZHNvJnt9dvNaPpSKVJE1+2VCUTCL59mSx olKWmnVYyfW9yCKGk+PwYi+6PpFjpzuXw/4HhShA42IJkLU2xWsWxyMUF6E4RwuIUzI8 xj6Z3PhKpn0OFiQqZ+aIMVIba4Y0/qbg3aZutLMEhZZmYCGP4Ij1judyG853giJgDf1h GCYrbmYQk9wuEC3tbaV7uO4gSbG+PY9FFkaabzQleDV/dkvQ3F4nq6uST9mkxcH3cBTy FlOw== X-Forwarded-Encrypted: i=1; AJvYcCV4vJbL0nJvytaURBbgTLjPClLTTR+IdUPv1Yx7CvNSIXQLBEe8OONm/oHfLHP24WoKopGYrpSMJESN@vger.kernel.org, AJvYcCXXN9SgkWnSR+WXSu8rZ18WTTvvNjlqc020C3GRFv1rzQa1xF96Fn1Vz4N7EYoz3lt1CNAGsfcZ3bbdrVA=@vger.kernel.org X-Gm-Message-State: AOJu0YyidhuoZMd36iKWz/xoKK5FPU/1JB883kR+bQmAhYSCT4H2cvkn 7sEyQTCXX8XQ8mMe8nrEURWAkad+8OTSnH1pppWSI6T4zuSIEuHI3P7DUw== X-Gm-Gg: ASbGncss8/+1R/r/9JAEiY9ARotJ/txUvY6aXIDmO60gmppIk4HnMlvcyVbk1ohkXwf aJLUUSX4OytvRVgl169Od4H6c4fBCqZ7aCduSVUY5OgreBhqRZr42o82bXgoQewa2EC4JmVLqqQ x1VWqMSz7Z38fVJyW+PudxNezFhrUTN2AhbbUdwVPRN8lFetsp1VbSe7rZNlnBegiyW5ktl2W6j yhQIyrwo+RxBb63Xw3566Hi/MtxVnyLAlMrl61i0h98EoSXruQGrDG01ol4qV6OmwTCUWR9QMEK ht9+92SQdkKK9JJuYKlMulmFyv3QOo5/ X-Google-Smtp-Source: AGHT+IGhCNmlhVmncAuG0EIzWBlQb0si8e3VTZqB0vnq48A5uaUKGEMTJ5Haico04YVyErcknQyj+A== X-Received: by 2002:a17:907:7fab:b0:ab7:9d30:3fea with SMTP id a640c23a62f3a-abc09aef73amr136512566b.29.1740100829828; Thu, 20 Feb 2025 17:20:29 -0800 (PST) Received: from foxbook (adtt173.neoplus.adsl.tpnet.pl. [79.185.231.173]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-aba5a4f4cb4sm1474755066b.118.2025.02.20.17.20.28 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Thu, 20 Feb 2025 17:20:29 -0800 (PST) Date: Fri, 21 Feb 2025 02:20:26 +0100 From: Michal Pecio To: Mathias Nyman Cc: Mathias Nyman , Greg Kroah-Hartman , Niklas Neronin , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v2 5/5] usb: xhci: Skip only one TD on Ring Underrun/Overrun Message-ID: <20250221022026.65c8e7fb@foxbook> In-Reply-To: <20250221021712.48c07fe0@foxbook> References: <20250210083718.2dd337c3@foxbook> <20250210084220.3e5414e9@foxbook> <7bb25848-c80e-4ba8-8790-8628951806b3@linux.intel.com> <20250221021712.48c07fe0@foxbook> Precedence: bulk X-Mailing-List: linux-usb@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 If skipping is deferred to events other than Missed Service Error itsef, it means we are running on an xHCI 1.0 host and don't know how many TDs were missed until we reach some ordinary transfer completion event. And in case of ring xrun, we can't know where the xrun happened either. If we skip all pending TDs, we may prematurely give back TDs added after the xrun had occurred, risking data loss or buffer UAF by the xHC. If we skip none, a driver may become confused and stop working when all its URBs are missed and appear to be "in flight" forever. Skip exactly one TD on each xrun event - the first one that was missed, as we can now be sure that the HC has finished processing it. Provided that one more TD is queued before any subsequent doorbell ring, it will become safe to skip another TD by the time we get an xrun again. Signed-off-by: Michal Pecio --- drivers/usb/host/xhci-ring.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 167ae7dfca47..d78b7877f65f 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -2844,8 +2844,21 @@ static int handle_tx_event(struct xhci_hcd *xhci, if (!ep_seg && usb_endpoint_xfer_isoc(&td->urb->ep->desc)) { skip_isoc_td(xhci, td, ep, status); - if (!list_empty(&ep_ring->td_list)) + + if (!list_empty(&ep_ring->td_list)) { + if (ring_xrun_event) { + /* + * If we are here, we are on xHCI 1.0 host with no + * idea how many TDs were missed or where the xrun + * occurred. New TDs may have been added after the + * xrun, so skip only one TD to be safe. + */ + xhci_dbg(xhci, "Skipped one TD for slot %u ep %u", + slot_id, ep_index); + return 0; + } continue; + } xhci_dbg(xhci, "All TDs skipped for slot %u ep %u. Clear skip flag.\n", slot_id, ep_index);