[PATCH 1/2] thunderbolt: Do not match XDomain responses to received packets

[Sven Neuhaus <sven-kernel@sven.de>]

What happens is that __tb_xdomain_response() queues a request that has
no response buffer. tb_cfg_request() schedules its work right after
sending it, and the request stays on ctl->request_queue until that work
dequeues it.

Until then tb_xdomain_match() can match a received error packet, or an
XDomain response with the same route and UUID, to this request. The
packet may really belong to a tb_xdomain_request() queued behind it.
tb_ctl_rx_callback() then calls schedule_work() again. If the first run
has already started, the work runs a second time after the request has
been released:

  refcount_t: underflow; use-after-free.
  WARNING: lib/refcount.c:28 at refcount_warn_saturate+0x59/0x90
  Workqueue: events tb_cfg_request_work [thunderbolt]
  Call Trace:
   tb_cfg_request_put+0x5a/0x70 [thunderbolt]
   tb_cfg_request_work+0x2e/0x50 [thunderbolt]
   process_one_work+0x19d/0x380

The match also reads req->request, which points to the caller's buffer.
That is often on the stack and gone by the time the packet arrives.

Such a request does not wait for anything, so never match it.

I've seen this 7.2.15 between two hosts connected with USB4 cables,
where an out-of-tree service driver exchanges XDomain requests and
responses in both directions. tb_xdomain_match() and
tb_cfg_request_work() are the same in mainline.

To test it, I added counters to ctl.c and ran ib_write_bw over that
driver in both directions. In 60 seconds a received packet matched a
request without a response buffer 527 times, and 6 times the work of
such a request was queued again after its callback had run and then
ran a second time. The refcount warning above is printed only once per
boot, so the log shows just the first one. With this patch and the
same load for 120 seconds, none of the counters moved, even with a
2-3 ms sleep added before tb_cfg_request_dequeue() to widen the window.

The patch was tested on 7.2.15 and build-tested on 7.3.3.
tb_xdomain_match() is unchanged in the thunderbolt.git fixes branch,
where the patch applies as is. Not tested: peers other than Linux and
the XDomain discovery protocol responses.

The analysis, the fix and the tests were done with an AI coding assistant.

Fixes: d1ff70241a27 ("thunderbolt: Add support for XDomain discovery protocol")
Cc: stable@vger.kernel.org
Signed-off-by: Sven Neuhaus <sven-kernel@sven.de>
---
 drivers/thunderbolt/xdomain.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/thunderbolt/xdomain.c b/drivers/thunderbolt/xdomain.c
index 563a8b65c854..64e2842b8b3f 100644
--- a/drivers/thunderbolt/xdomain.c
+++ b/drivers/thunderbolt/xdomain.c
@@ -90,6 +90,13 @@ bool tb_is_xdomain_enabled(void)
 static bool tb_xdomain_match(const struct tb_cfg_request *req,
 			     const struct ctl_pkg *pkg)
 {
+	/*
+	 * A request without a response buffer is complete once it has
+	 * been sent, and req->request may no longer be valid.
+	 */
+	if (!req->response)
+		return false;
+
 	switch (pkg->frame.eof) {
 	case TB_CFG_PKG_ERROR:
 		return true;
-- 
2.47.3