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

Message-ID:
 <ME0P300MB0553E15D02A52DB482496B2CA6182@ME0P300MB0553.AUSP300.PROD.OUTLOOK.COM>
Date: Tue, 14 Jan 2025 11:46:02 +0800
From: Junzhong Pan <panjunzhong@...look.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,

Thanks for your quick reply.

On 2025/1/14 3:22, Maciej Żenczykowski Wrote:
> Looking at https://github.com/microsoft/NCM-Driver-for-Windows
> 
> commit ded4839c5103ab91822bfde1932393bbb14afda3 (tag:
> windows_10.0.22000, origin/master)
> Author: Brandon Jiang <jiangyue@...rosoft.com>
> Date:   Mon Oct 4 14:30:44 2021 -0700
> 
>     update NCM to Windows 11 (21H2) release, built with Windows 11
> (22000) WDK and DMF v1.1.82
> 
> -- vs previous change to host/device.cpp
> 
> commit 40118f55a0843221f04c8036df8b97fa3512a007 (tag:
> windows_10.0.19041, origin/release_2004)
> Author: Brandon Jiang <jiangyue@...rosoft.com>
> Date:   Sun Feb 23 19:53:06 2020 -0800
> 
>     update NCM to 20H1 Windows release, built with 20H1 WDK and DMF v1.1.20
> 
> it introduced
> 
>     if (bufferRequest->TransferLength < bufferRequest->BufferLength &&
>         bufferRequest->TransferLength %
> hostDevice->m_DataBulkOutPipeMaximumPacketSize == 0)
>     {
>         //NCM spec is not explicit if a ZLP shall be sent when
> wBlockLength != 0 and it happens to be
>         //multiple of wMaxPacketSize. Our interpretation is that no
> ZLP needed if wBlockLength is non-zero,

In NCM10, 3.2.2 dwBlockLength description, it states:
> If exactly dwNtbInMaxSize or dwNtbOutMaxSize bytes are sent, and the 
> size is a multiple of wMaxPacketSize for the given pipe, then no ZLP
> shall be sent.
I don't know if its a Microsoft's problem or really **not explicit**.
Maybe most of the device implementations treat the incoming data as a
stream and do contiguous parsing on it.

>         //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 rely on ZLP as long
> as the wBlockLength is multiple of wMaxPacketSize.
>         //To deal with such devices, we pad an extra 0 at end so the
> transfer is no longer multiple of wMaxPacketSize
> 
>         bufferRequest->Buffer[bufferRequest->TransferLength] = 0;
>         bufferRequest->TransferLength++;
>     }
> 
> Which I think is literally the fix for this bug you're reporting.
> That 'fix' is what then caused us to add the patch at the top of this thread.
> 
> So that fix was present in the very first official Win11 release
> (build 22000), but is likely not present in any Win10 release...

As you mentioned before to fix it in the gadget side, it seems very 
complicated, maybe we need a extra skb with size=ntb_size as an intermediate
buffer to move around those ntb data before parsing it, but may (or may not)
lead to performance drop. Any other idea?

Do you think hacking in the gadget side to fix this compatible issue is
a good idea consider that there are still a large number of users using
Win10?

(Though Win10 will reach end of support on October 14, 2025, but people
may still use it for a long time since many PCs in good condition cannot
install win11.)

Best Regards