netdev - Re: [PATCH net 0/3] Add I2C bus lock for Wangxun

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: <d91674af-1682-4efe-ad15-bd64f871c1de@lunn.ch>
Date: Thu, 29 Aug 2024 17:27:49 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Jiawen Wu <jiawenwu@...stnetic.com>
Cc: andi.shyti@...nel.org, jarkko.nikula@...ux.intel.com,
	andriy.shevchenko@...ux.intel.com, mika.westerberg@...ux.intel.com,
	jsd@...ihalf.com, davem@...emloft.net, edumazet@...gle.com,
	kuba@...nel.org, pabeni@...hat.com, rmk+kernel@...linux.org.uk,
	linux-i2c@...r.kernel.org, netdev@...r.kernel.org,
	mengyuanlou@...-swift.com, duanqiangwen@...-swift.com
Subject: Re: [PATCH net 0/3] Add I2C bus lock for Wangxun

> > O.K, that is bad. The SFP is totally unreliable...
> > 
> > You however have still not answered my question. What is the firmware
> > accessing? How does it handle pages?
> >
> > The hack you have put in place is per i2c transaction. But accessing
> > pages is likely to be multiple transactions. One to change the page,
> > followed by a few reads/writes in the new page, then maybe followed by
> > a transactions to return to page 0.
> 
> Do you mean the bus address A0 or A2? Firmware accesses I2C just like driver,
> but it only change the page once per full transaction, during a possession of
> the semaphore.  What you fear seems unlikely to happen.

What sort of SFP is this? QSFP byte 127 selects the page for addresses
128-255. Paged 0 and 3 are mandatory, pages 1 and 2 are optional.

SFP+ also uses byte 127 in the same way:

10.3 Optional Page Select Byte [Address A2h, Byte 127]

In order to provide memory space for DWDM and CDR control functions
and for other potential extensions, multiple Pages can be defined for
the upper half of the A2h address space. At startup the value of byte
127 defaults to 00h, which points to the User EEPROM. This ensures
backward compatibility for transceivers that do not implement the
optional Page structure. When a Page value is written to byte 127,
subsequent reads and writes to bytes 128-255 are made to the relevant
Page.

This specification defines functions in Pages 00h-02h. Pages 03-7Fh
are reserved for future use. Writing the value of a non-supported Page
shall not be accepted by the transceiver. The Page Select byte shall
revert to 0 and read / write operations shall be to the unpaged A2h
memory map.

ethtool allows you to access more than page 0.

ethtool -m|--dump-module-eeprom|--module-info devname [raw on|off]
        [hex on|off] [offset N] [length N] [page N] [bank N] [i2c N]

> > I think your best solution is to simply take the mutex and never
> > release it. Block your firmware from accessing the SFP.
> 
> Firmware accesses the SFP in order to provide information to the BMC.
> So it cannot simply be blocked.

Then you have a design problem. And i don't think locking the I2C bus
per transaction is sufficient.

	Andrew