linux-kernel - Re: [PATCH v2] usb: gadget: functioni: Fix a oob problem in rndis

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: <tencent_19AE390D05232D1C97E336C05F35A9F1BD05@qq.com>
Date: Fri, 11 Jul 2025 11:46:35 +0800
From: jackysliu <1972843537@...com>
To: gregkh@...uxfoundation.org
Cc: 1972843537@...com,
	linux-kernel@...r.kernel.org,
	linux-usb@...r.kernel.org,
	viro@...iv.linux.org.uk
Subject: Re: [PATCH v2] usb: gadget: functioni: Fix a oob problem in rndis

On Thu, 10 Jul 2025 14:19:47 +0200, greg k-h wrote

>In digging in this further, I don't see how this actually changes
>anything.  BufLength is used for nothing that I can determine, except in
>some debugging code that is always compiled out (i.e. you can NOT enable
>it unless you modify the kernel source.)
>
>So what exactly is this check checking?
>
>I can see that we really should be checking if the buffer is too small,
>but that's not what you are doing here at all.
>
>And all this buffer is used for is to read a 32bit value out of, so
>verifying that the buffer really is big enough to hold that value SHOULD
>be what we do here, not check to see if the buffer is too big.
>
>Also, you can't trust that BufLength is even correct as it comes from
>the other side, right?  Because of that, we should just be ignoring it
>entirely and verifying that the message size really is as big as the
>structure is supposed to be.  But that means passing down the message
>size to the lower layers here, which gets into the issues that I have
>raised before many years ago about this protocol, and this
>implementation of this protocol.  I.e, it is COMPLETELY INSECURE and
>should ONLY be used on systems where you trust both sides of the wire.
>
>Again, how was this change tested?  And what exactly does it fix?  I'm
>missing how this change is going to actually catch anything, can you
>spell it out in detail for me?

BufOffset + BufLength can exceed buffer size even if each is < RNDIS_MAX_TOTAL_SIZE
oob is triggered by a function call to gen_ndis_set_resp.
I supposed to add an additional boundry check to avoid this issue.But That 
doesn't seem to be enough to fix the bug.I'll try to figure it out.

Thanks

Siyang Liu