Received-SPF: pass (google.com: domain of linux-kernel+bounces-138273-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Wed, 10 Apr 2024 11:46:01 +0200
From: =?UTF-8?B?TWljaGHFgg==?= Pecio <michal.pecio@gmail.com>
To: Mathias Nyman <mathias.nyman@linux.intel.com>
Cc: Paul Menzel <pmenzel@molgen.mpg.de>, Mathias Nyman
 <mathias.nyman@intel.com>, LKML <linux-kernel@vger.kernel.org>,
 linux-usb@vger.kernel.org, Niklas Neronin <niklas.neronin@linux.intel.com>
Subject: Re: xhci_hcd 0000:00:14.0: ERROR Transfer event TRB DMA ptr not
 part of current TD ep_index 1 comp_code 1
Message-ID: <20240410114601.0e25a46d@foxbook>
In-Reply-To: <82113c7d-0405-ba11-94d9-5673593cec50@linux.intel.com>
References: <58bca6f2-797a-4e20-a476-2294309afdd5@molgen.mpg.de>
	<20240405113247.743e34b2@foxbook>
	<7090d3af-18ce-40e1-8ac2-bf18152e5c4a@molgen.mpg.de>
	<20240406183659.3daf4fa0@foxbook>
	<c57f2116-8c42-44fb-9c32-6115ad88f914@molgen.mpg.de>
	<20240407142542.036fb02f@foxbook>
	<1f64af9a-0618-a7da-4acc-f043b6580308@linux.intel.com>
	<20240408210541.771253ff@foxbook>
	<82113c7d-0405-ba11-94d9-5673593cec50@linux.intel.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

> Driver can cope with these extra events, but if this is common we
> should probably handle it silently and not concern users with that
> ERROR message.

The error message in itself is harmless, it means the driver gets an
event it doesn't know how to handle and ignores it. Further events are
processed normally (in this specific case).

What's problematic is that the controller is apparently still working
on a TD which the driver considers to be finished already. The driver
can overwrite the TD and reuse its data buffer for other transfers,
while the hardware may still need the original TD for proper operation
and, if we are very unlucky, could attempt DMA to/from the data buffer,
causing data corruption or information leak to a malicious USB dongle.

For all we know, Paul's buggy chipset may not only be confirming the
transfer twice, but really performing it twice for some stupid reason.


> We are actually at the moment looking at improving handle_tx_event()
> with Niklas (cc), and will take this into consideration.

Given the number of bugs so far, maybe it would make sense to count
transfer ring slots of the last completed TD as still "in use" until
the next TD is known to at least begin executing.

Unfortunately, "quarantining" URB data buffers in similar manner would
be harder AFAIK.

I recently found one more bug of this kind: the Etron EJ168 controller
produces two events for failed single-TRB isochronous IN transfers -
one event indicating the failure, and then a "success". The full extent
of the bug (does it affect OUT or non-isoch, what happens on multi-TRB)
is unknown because the controller is very prone to crashing under my
workloads, which doesn't help debugging.

Regards,
Michal