netdev - RE: [PATCH v1 net-next] lan743x: Provide Read/Write Access to on chip OTP

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: <CY4PR1101MB23423D568D31FF26860B1BC7FA9F0@CY4PR1101MB2342.namprd11.prod.outlook.com>
Date:   Mon, 21 Jan 2019 20:30:26 +0000
From:   <Bryan.Whitehead@...rochip.com>
To:     <andrew@...n.ch>
CC:     <davem@...emloft.net>, <netdev@...r.kernel.org>,
        <UNGLinuxDriver@...rochip.com>
Subject: RE: [PATCH v1 net-next] lan743x: Provide Read/Write Access to on chip
 OTP

> > > Hi Bryan
> > >
> > > It would be good to explain what is wrong with the current code,
> > > which allows you to select between the OTP and the EEPROM at write
> time.
> >
> > Hi Andrew,
> >
> > The current code does not allow OTP read access.
> > Plus the current code places unreasonable restrictions on OTP write
> operations.
> > Such as requiring offset == 0, length == 512,
> >     and data[0] == 0xF3, which is the signature used to indicate the first OTP
> block is valid.
> >     However if the user wanted to use the second block, then data[0] should
> be 0xF7,
> >         And data should start at offset 513.
> >     Also if you want to permanently invalidate the OTP then data[0] should
> be 0xFF.
> > This patch allows the user the program OTP with any offset, length and
> data.
> > And read access is necessary to confirm what is currently in the OTP.
> >
> > Would you like me to submit a new patch containing this information?
> 
> Hi Bryan
> 
> Yes, please indicate why the patch is needed.

OK

> 
> > > Is the EEPROM mandatory? Could there be designs without an EEPROM?
> > > When setting the private flag here, should you probe to see if there
> > > actually is an EEPROM and return -ENODEV if it is missing.
> >
> > EEPROM is not mandatory. If EEPROM is not attached or contains invalid
> > data, the lan743x will attempt to load configuration from on chip OTP
> memory.
> >
> > To be honest, I'm not sure how to do a EEPROM probe with out doing a
> > Write and read back test. And it is not recommended to do that due to
> > limited write cycles which cause EEPROM damage.
> >
> > The current code does not do an EEPROM probe, so I don't see why the
> > new code should. And we assume that the user can diagnose EEPROM
> > presence using existing read/write operations.
> 
> I would also put some text in the commit message to it is possible to swap to
> the EEPROM using the private setting, even when the EEPROM is not
> present. The failure will only happen later. With the current code, the failure
> is going to happen immediately.

OK, I'll submit a new patch soon.