linux-kernel - Re: [PATCH net-next] net: usb: aqc111: fix error handling of usbnet read calls

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: <dab3eb18-dc98-45bf-a81a-56cec78eaac0@redhat.com>
Date: Mon, 26 May 2025 15:54:17 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Andrew Lunn <andrew@...n.ch>,
 Nikita Zhandarovich <n.zhandarovich@...tech.ru>
Cc: Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, linux-usb@...r.kernel.org,
 netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
 syzbot+3b6b9ff7b80430020c7b@...kaller.appspotmail.com,
 lvc-project@...uxtesting.org
Subject: Re: [PATCH net-next] net: usb: aqc111: fix error handling of usbnet
 read calls

On 5/21/25 2:34 PM, Andrew Lunn wrote:
> On Tue, May 20, 2025 at 02:32:39PM +0300, Nikita Zhandarovich wrote:
>> Syzkaller, courtesy of syzbot, identified an error (see report [1]) in
>> aqc111 driver, caused by incomplete sanitation of usb read calls'
>> results. This problem is quite similar to the one fixed in commit
>> 920a9fa27e78 ("net: asix: add proper error handling of usb read errors").
>>
>> For instance, usbnet_read_cmd() may read fewer than 'size' bytes,
>> even if the caller expected the full amount, and aqc111_read_cmd()
>> will not check its result properly. As [1] shows, this may lead
>> to MAC address in aqc111_bind() being only partly initialized,
>> triggering KMSAN warnings.
> 
> It looks like __ax88179_read_cmd() has the same issue? Please could
> you have a look around and see if more of the same problem exists.
> 
> Are there any use cases where usbnet_read_cmd() can actually return
> less than size and it not being an error? Maybe this check for ret !=
> size can be moved inside usbnet_read_cmd()?

Judging from __usbnet_read_cmd() implementation, it's actually expected
that such helper could return a partial copy. The centralized check
could possibly break some users, I think check the return value on a
per-device basis is safer.

Side note: the patch should have targeted the 'net' tree as the blamed
commit is present there, given we are now in the merge window and
net-next PR is somewhat upcoming, applying it to net-next will yield the
same result.

/P