linux-kernel - Re: [PATCH net v2] net: usb: lan78xx: fix use of improperly initialized dev->chipid in lan78xx

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: <578cc11f-654d-44fb-829a-ae6421863d50@kernel.org>
Date: Wed, 15 Oct 2025 13:00:52 -0600
From: Khalid Aziz <khalid@...nel.org>
To: I Viswanath <viswanathiyyappan@...il.com>
Cc: Thangaraj.S@...rochip.com, Rengarajan.S@...rochip.com,
 UNGLinuxDriver@...rochip.com, andrew+netdev@...n.ch, davem@...emloft.net,
 edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
 netdev@...r.kernel.org, linux-usb@...r.kernel.org,
 linux-kernel@...r.kernel.org, skhan@...uxfoundation.org,
 linux-kernel-mentees@...ts.linux.dev, david.hunter.linux@...il.com
Subject: Re: [PATCH net v2] net: usb: lan78xx: fix use of improperly
 initialized dev->chipid in lan78xx_reset

On 10/15/25 10:51 AM, I Viswanath wrote:
> On Wed, 15 Oct 2025 at 21:25, Khalid Aziz <khalid@...nel.org> wrote:
> 
>> How did you determine this is the commit that introduced this bug?
>>
>>   From what I can see, commit a0db7d10b76e does not touch lan78xx_reset()
>> function. This bug was introduced when devid was replaced by chipid
>> (commit 87177ba6e47e "lan78xx: replace devid to chipid & chiprev") or
>> even earlier when the order of calls to lan78xx_init_mac_address() and
>> lan78xx_read_reg() was introduced in lan78xx_reset() depending upon if
>> lan78xx_init_mac_address() at that time used devid in its call sequence
>> at the time.
> 
> The commit a0db7d10b76e introduced the dependency on devid to
> lan78xx_read_raw_eeprom() and
> lan78xx_read_eeprom() and ultimately lan78xx_init_mac_address() and
> lan78xx_reset()
> 
> In lan78xx_init_mac_address()
> 
> Only lan78xx_read_eeprom() depends on devid as
> 
> lan78xx_read_reg() and lan78xx_write_reg() do not use devid
> 
> lan78xx_read_otp() depends on lan78xx_read_raw_otp() which depends
> only on lan78xx_write_reg() and lan78xx_read_reg()
> and hence doesn't use devid either
> 
> is_valid_ether_addr(), random_ether_addr() and ether_addr_copy() are
> net core functions and do not care about driver specific data
> 
> The devid read exists in this commit (was added in ce85e13ad6ef4)
> 
> a0db7d10b76e was supposed to move the devid read before the
> lan78xx_init_mac_address() because of the newly added
> dependency but it was a tricky detail that the author failed to see
> 
> Thanks,
> I Viswanath

Ah, I see. That makes sense.

--
Khalid