linux-kernel - cdns3 uvc first ISO parkage lost problem

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZTvhEl+JcnhJXcrl@lizhi-Precision-Tower-5810>
Date:   Fri, 27 Oct 2023 12:10:58 -0400
From:   Frank Li <Frank.li@....com>
To:     pawell@...ence.com, peter.chen@...nel.org, rogerq@...nel.org,
        a-govindraju@...com, gregkh@...uxfoundation.org,
        linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org,
        imx@...ts.linux.dev
Subject: cdns3 uvc first ISO parkage lost problem

hi Pawel Laszczak

Recently, I met the problem when use uvc. UVC report jpg header error.

Basic reproduce steps.
Gadget side:
1 - 
	https://gist.github.com/kbingham/c39c4cc7c20882a104c08df5206e2f9f?permalink_comment_id=3270713
	uvc-gadget.sh start
2 -
	https://git.ideasonboard.org/uvc-gadget.git
	uvc-gadget -i test.jpg

Host side:
	https://github.com/thekvs/uvccapture2
	uvccapture2 --device /dev/video0  --resolution 640x360 --count 1 --result 8qxp.jpeg

	It will report jpeg header error.

After debugs, I found two problem. 

Problem 1, sg is enabled. so uvc driver will use sg. each package include
two trb,  trb0 is 8bytes header, trb1 is 1016bytes. total 1024.

num_trb here is wrong.
it should be 
	num_trb = priv_ep->interval * request->num_mapped_sgs.

because priv_ep->interval is 1, I just simple set to request->num_mapped_sg
as below patch. USB analyer show one whole 1024 ISO package sent out as
expectation although document said only support one TD when use ISO (Maybe
my doc is too old).

diff --git a/drivers/usb/cdns3/cdns3-gadget.c b/drivers/usb/cdns3/cdns3-gadget.c
index 69a44bd7e5d02..8cc99a885883f 100644
--- a/drivers/usb/cdns3/cdns3-gadget.c
+++ b/drivers/usb/cdns3/cdns3-gadget.c
@@ -1125,10 +1125,7 @@ static int cdns3_ep_run_transfer(struct cdns3_endpoint *priv_ep,
        struct scatterlist *s = NULL;
        bool sg_supported = !!(request->num_mapped_sgs);

-       if (priv_ep->type == USB_ENDPOINT_XFER_ISOC)
-               num_trb = priv_ep->interval;
-       else
-               num_trb = sg_supported ? request->num_mapped_sgs : 1;
+       num_trb = sg_supported ? request->num_mapped_sgs : 1;

        if (num_trb > priv_ep->free_trbs) {
                priv_ep->flags |= EP_RING_FULL;

*** Problem 2 ***

According to doc and my observation, looks like hardware fetch data into
FIFO when get SOF, then transfer data when get IN token. Each SOF will
increase TRB regardless it is ready or not.

When gadget complete equeue ISO data, so SOF will increase TRB regardless
if there are IN token. 

   SOF       SOF       SOF     SOF  IN    SOF ....
      TRB0      TRB1      TRB2      TRB3  ...

Host may start get data at some time after gadget queue data.

So TRB0..2 data will be lost.

If it is audio data, it should be okay. But for uvc, it is jpeg header, so
host side report error.

I checked dwc gadget driver, which start equeue ISO data only get NYET.

I check cdns spec, there are ISOERR. But it is never happen. According to
document, ISOERR should issue when IN token and FIFO no data. 

I tried below method
	1.  Delay queue TRB, but no ISOERR.
	2.  queue a lenght 0 TRB,but no ISOERR 

My question is how to delay queue TRB to ISO IN token really happen to
avoid lost JPEG header.

Frank