netdev - RE: [PATCH net-next 5/5] qed*: Add support for read/write of eeprom

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: <CO2PR11MB0088716755CC57276455ADA4976B0@CO2PR11MB0088.namprd11.prod.outlook.com>
Date:	Mon, 18 Apr 2016 03:59:42 +0000
From:	Yuval Mintz <Yuval.Mintz@...gic.com>
To:	David Miller <davem@...emloft.net>
CC:	netdev <netdev@...r.kernel.org>,
	Sudarsana Kalluru <Sudarsana.Kalluru@...gic.com>
Subject: RE: [PATCH net-next 5/5] qed*: Add support for read/write of eeprom

> > There's quite a bit of `magic' here [based on the ethtool `magic'
> > field] which is retained by the user application responsible for
> > interacting with the driver for the purpose of upgrading the nvram
> > image.
> 
> This is not how the 'magic' value of the eeprom structure is specified to be used,
> please use it the way it is supposed to be used rather than inventing semantics
> which only apply to your device.
> 
> The entire point of defining a common interface for device driver interface
> interaction with the user is exactly so that driver authors don't do things like this.

While I obviously get what you're writing, I actually think this is the
more revealing API option.

I.e., the bottom line is that write to this device's nvram is complex in
nature and requires interacting with the management firmware.
The other immediate option would have been to implement this
in driver as simple read/write toward the management firmware,
and let the management firmware analyze what it needs to do with
the data based on hidden meta-data inside the buffer written by the
application.

While this might be less 'extending' of the ethtool API, it's a security-
in-obscurity kind of API, where a reviewer can't possibly know how to
perform read/write from reading the driver code.

The third option I see is to completely ditch this, stating that although
We obviously CAN set the non-volatile memory we CAN'T do it with the
regular API, and to move into doing this via some proprietary API such
as debugfs.

Thanks,
Yuval