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

Message-ID: <CANP3RGc_SBROWVA2GMaN41mzCU28wGtQzT5qmSKcYsYDY03G5g@mail.gmail.com>
Date: Fri, 10 Jan 2025 11:00:19 -0800
From: Maciej Żenczykowski <maze@...gle.com>
To: Junzhong Pan <panjunzhong@...look.com>
Cc: quic_kriskura@...cinc.com, gregkh@...uxfoundation.org, 
	hgajjar@...adit-jv.com, 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

On Thu, Jan 9, 2025 at 11:37 PM Junzhong Pan <panjunzhong@...look.com> wrote:
>
> Hi everyone,
>
> I recently switch to f_ncm with Windows 10 since rndis 's safety issue.
> (the Windows 10 driver version is 10.0.19041.1 2009/4/21)
>
> It seems Windows 10 ncm driver won't send ZLP to let udc properly
> separate the skbs.
>
> On Mon, 5 Feb 2024 13:16:50 +0530 Krishna Kurapati wrote:
> > According to Windows driver, no ZLP is needed if wBlockLength is
> non-zero,
> > because the non-zero wBlockLength has already told the function side the
> > size of transfer to be expected. However, there are in-market NCM devices
> > that rely on ZLP as long as the wBlockLength is multiple of
> wMaxPacketSize.
> > To deal with such devices, it pads an extra 0 at end so the transfer
> is no
> > longer multiple of wMaxPacketSize.
>
> I do the iperf3 testing cause gadget constantly report similar error after
> a litle modification to get more concrete info:
>
> [  174] configfs-gadget.0: to process=512, so go to find second NTH
> from: 15872
> [  174] FIND NEXT NTH HEAD:000000006c26a12c: 6e 63 6d 68 10 00 86 16 b0
> 3b 00 00 48 3b 00 00 00 00 52 34 fc 5f 90 fd ca 40 c1 f4 f4 6e 08 00
> [  174] configfs-gadget.0: Wrong NDP SIGN of this ndp index: 15176, skb
> len: 16384, ureq_len: 16384, this wSeq: 5766
> [  174] NDP HEAD:00000000298f3cab: 2b 12 48 8f 12 ce 3c c8 d7 39 c0 0d
> 15 cf 86 14 17 4a 91 85 db df ad 87 f0 35 0d 76 ad 4d 4d 74
> [  174] NTH of this NDP HEAD:00000000af9fbfc9: 6e 63 6d 68 10 00 85 16
> 00 3e 00 00 90 3d 00 00 00 00 52 34 fc 5f 90 fd ca 40 c1 f4 f4 6e 08 00
> [  174] configfs-gadget.0: Wrong NTH SIGN, skblen 14768, last wSequence:
> 5766, last dgram_num: 11, ureqlen: 16384
> [  174] HEAD:00000000b1a72bfc: 3f 98 a6 8e 17 f8 bb 29 07 b8 da 13 7f 20
> 80 8e 77 ca 32 07 ac 71 b8 8d 84 03 d7 1b 96 9b c4 fa
>
>
> Lecroy shows the wSequence=5765 have 10 Datagram consisting a 31*512
> bytes=15872 bytes OUT Transfer but have no ZLP:
>
> OUT Transfer wSequence=5765
>         NTH32 Datagrams: 1514B * 8 + 580B NDP32
>         Transfer length: 512B * 31
>         NO ZLP
> OUT Transfer wSequence=5766
>         NTH32 Datagrams: 1514B * 8 NDP32
>         Transfer length: 512B * 29  + 432
>
> This lead to a result that the first givebacked 16K skb correponding to
> a usb_request contains two NTH but not complete:
>
> USB Request 1 SKB 16384B
>         (NTH32) (Datagrams) (NDP32) | (NTH32) (Datagrams piece of wSequence=5766)
> USB Request 2 SKB 14768B
>         (Datagrams piece of wSequence=5766) (NDP32)
>
>  From the context, it seems the first report of Wrong NDP SIGN is caused
> by out-of-bound accessing, the second report of Wrong NTH SIGN is caused
> by a wrong beginning of NTB parsing.
>
> Do you have any idea how can this be fixed so that the ncm compatibility
> is better for windows users.
>
> Best Regards,
> Pan

Could you clarify which Linux Kernel you're testing against?
Either X.Y.Z version or some git kernel sha1 (not including your debug
code of course).

Could you provide some pcap of the actual usb frames?
Or perhaps describe better the problem, because I'm not quite
following from your email.
(I'm not sure if the problem is what windows is sending, or how Linux
is parsing it)

I *think* what you're saying is that wSequence=5765 & 5766 are being
treated as a single ncm message due to their being a multiple of 512
in the former, not followed by a ZLP?  I thought that was precisely
when microsoft ncm added an extra zero byte...

What's at the end of 5755?  Padding? No padding?
Is there an NTH32 header in 5766?  Should there be?

--
Maciej Żenczykowski, Kernel Networking Developer @ Google