linux-kernel - Re: [PATCH v3] usb: gadget: ncm: Avoid dropping datagrams of properly parsed NTBs

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>] [day] [month] [year] [list]

Message-Id: <4C771A46-5372-488E-B31F-67262CDB95D6@icloud.com>
Date: Mon, 13 Jan 2025 21:06:42 +0800
From: 潘俊中 <a1134123566@...oud.com>
To: Maciej Żenczykowski <maze@...gle.com>
Cc: quic_kriskura@...cinc.com, gregkh@...uxfoundation.org,
 linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org,
 quic_jackp@...cinc.com, quic_ppratap@...cinc.com, quic_wcheng@...cinc.com,
 stable@...r.kernel.org
Subject: Re: [PATCH v3] usb: gadget: ncm: Avoid dropping datagrams of properly parsed NTBs

Hi Maciej,

On 2025/1/13 1:49, Maciej Żenczykowski Wrote:
> (a) I think this looks like a bug on the sending Win10 side, rather
> than a parsing bug in Linux,
> with there being no ZLP, and no short (<512) frame, there's simply no
> way for the receiver to split at the right spot.
> 
> Indeed, fixing this on the Linux/parsing side seems non-trivial...
> I guess we could try to treat the connection as simply a serial
> connection (ie. ignore frame boundaries), but then we might have
> issues with other senders...
> 
> I guess the most likely 'correct' hack/fix would be to hold on to the
> extra 'N*512' bytes (it doesn't even have to be 1, though likely the N
> is odd), if it starts with a NTH header...
Make sence, it seems we only need to save the rest data beside
dwBlockLength for next unwrap if a hack is acceptable, otherwise I may
need to check if a custom host driver for Windows10 user feasible.

I didn't look carefully into the 1byte and padding stuff with Windows11
host yet, I will take a look then.

> (b) I notice the '512' not '1024', I think this implies a USB2
> connection instead of USB3
> -- could you try replicating this with a USB3 capable data cable (and
> USB3 ports), this should result in 1024 block size instead of 512.
> 
> I'm wondering if the win10 stack is avoiding generating N*1024, but
> then hitting N*512 with odd N...
Yes, I am using USB2.0 connection to better capture the crime scene.

Normally the OUT transfer on USB3.0 SuperSpeed connection comes with a bunch
of 1024B Data Pakcet along with a Short Packet less than 1024B in the end from
the Lecroy trace.

It's also reproducible on USB3.0 SuperSpeed connection using dwc3 controller,
but it will cost more time and make it difficult to capture the online data
(limited tracer HW buffer), I can try using software tracing or custom logs
later:

[  5]  26.00-27.00  sec   183 MBytes  1.54 Gbits/sec
[  5]  27.00-28.00  sec   182 MBytes  1.53 Gbits/sec
[  206.123935] configfs.gadget.2: Wrong NDP SIGN
[  206.129785] configfs.gadget.2: Wrong NTH SIGN, skblen 12208
[  206.136802] HEAD:0000000004f66a88: 80 06 bc f9 c0 a8 24 66 c0 a8 24 65 f7 24 14 51 aa 1a 30 d5 01 f8 01 26 50 10 20 14 27 3d 00 00
[  5]  28.00-29.00  sec   128 MBytes  1.07 Gbits/sec
[  5]  29.00-30.00  sec   191 MBytes  1.61 Gbits/sec
> 
> Presumably '512' would be '64' with USB1.0/1.1, but I guess finding a
> USB1.x port/host to test against is likely to be near impossible...
> 
> I'll try to see if I can find the source of the bug in the Win
> driver's sources (though based on it being Win10 only, may need to
> search history)
> It's great if you can analyze from the host driver.

I didn't know if the NCM driver open-sourced on github by M$ is the correspond
version. They said that only Win 11 officially support NCM in the issue on github
yet they do have a built-in driver in Win10 and 2004 tag there in the repo.