linux-kernel - Re: [PATCH 1/4] net: sfp: add workaround for Realtek RTL8672 and RTL9601C chips

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: <20201231170039.zkoa6mij3q3gt7c6@pali>
Date:   Thu, 31 Dec 2020 18:00:39 +0100
From:   Pali Rohár <pali@...nel.org>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Russell King - ARM Linux admin <linux@...linux.org.uk>,
        Heiner Kallweit <hkallweit1@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Marek Behún <kabel@...nel.org>,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/4] net: sfp: add workaround for Realtek RTL8672 and
 RTL9601C chips

On Thursday 31 December 2020 16:30:33 Andrew Lunn wrote:
> On Thu, Dec 31, 2020 at 01:14:10PM +0100, Pali Rohár wrote:
> > On Wednesday 30 December 2020 19:09:58 Russell King - ARM Linux admin wrote:
> > > On Wed, Dec 30, 2020 at 06:43:07PM +0100, Pali Rohár wrote:
> > > > On Wednesday 30 December 2020 18:13:15 Andrew Lunn wrote:
> > > > > Hi Pali
> > > > > 
> > > > > I have to agree with Russell here. I would rather have no diagnostics
> > > > > than untrustable diagnostics.
> > > > 
> > > > Ok!
> > > > 
> > > > So should we completely skip hwmon_device_register_with_info() call
> > > > if (i2c_block_size < 2) ?
> > > 
> > > I don't think that alone is sufficient - there's also the matter of
> > > ethtool -m which will dump that information as well, and we don't want
> > > to offer it to userspace in an unreliable form.
> > 
> > Any idea/preference how to disable access to these registers?
> 
> Page A0, byte 92:
> 
> "Diagnostic Monitoring Type" is a 1 byte field with 8 single bit
> indicators describing how diagnostic monitoring is implemented in the
> particular transceiver.
> 
> Note that if bit 6, address 92 is set indicating that digital
> diagnostic monitoring has been implemented, received power
> monitoring, transmitted power monitoring, bias current monitoring,
> supply voltage monitoring and temperature monitoring must all be
> implemented. Additionally, alarm and warning thresholds must be
> written as specified in this document at locations 00 to 55 on
> two-wire serial address 1010001X (A2h) (see Table 8-5).
> 
> Unfortunately, we cannot simply set sfp->id.ext.diagmon to false,
> because it can also be used to indicate power, software reading of
> TX_DISABLE, LOS, etc. These are all single bytes, so could be returned
> correctly, assuming they have been implemented according to the spec.
> 
> Looking at sfp_module_info(), adding a check for i2c_block_size < 2
> when determining what length to return. ethtool should do the right
> thing, know that the second page has not been returned to user space.

But if we limit length of eeprom then userspace would not be able to
access those TX_DISABLE, LOS and other bits from byte 110 at address A2.

Problematic two-byte values are in byte range 96-109 at address A2.
Therefore before byte 110.