netdev - Re: + e1000e-prevent-corruption-of-eeprom-nvm.patch added to -mm tree

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]

Date:	Wed, 3 Sep 2008 01:44:56 +0100
From:	Ben Hutchings <bhutchings@...arflare.com>
To:	Zachary Amsden <zach@...are.com>
Cc:	"Brandeburg, Jesse" <jesse.brandeburg@...el.com>,
	Qicheng Christopher Li <chrisl@...are.com>,
	"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
	"arvidjaar@...l.ru" <arvidjaar@...l.ru>,
	"Allan, Bruce W" <bruce.w.allan@...el.com>,
	"jeff@...zik.org" <jeff@...zik.org>,
	"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>,
	"Ronciak, John" <john.ronciak@...el.com>,
	"Waskiewicz Jr, Peter P" <peter.p.waskiewicz.jr@...el.com>,
	Pratap Subrahmanyam <pratap@...are.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"mm-commits@...r.kernel.org" <mm-commits@...r.kernel.org>
Subject: Re: + e1000e-prevent-corruption-of-eeprom-nvm.patch added to -mm tree

Zachary Amsden wrote:
> On Tue, 2008-09-02 at 14:58 -0700, Brandeburg, Jesse wrote:
[...]
> > VMWare issues really do not apply to the Linux kernel, and unless the
> > architecture of ethtool->ioctl->rtnl->e1000e has changed without me
> > noticing.  In that case every driver that used ethtool would be broken.
> 
> No, any drivers that have a state machine consisting of a base register
> that is used to index which datum is read from device EEPROM by the data
> register would be broken.
> 
> This is not a VMware issue, its a fundamental hardware synchronization
> issue caused by the dependence of data read/write on a device register.
> Does the net layer protect against e1000 issuing simultaneous EEPROM
> read / write?

All ethtool commands are serialised by the RTNL as Jesse said above.

> I have no idea.  It probably should.  Should the driver
> take extra precautions of its own to ensure this on non hot-paths, to
> avoid corrupting EEPROM memory and resulting in a non-functional NIC?
> 
> Absolutely.
> 
> From casual inspection it looks like set_mtu is done under dev_baselock, while ethtool is done under rtnl_lock.
> 
> Setting the MTU internally reads the EEPROM, which could contend with and corrupt and eeprom operations.
 
No, MTU changes are done by another ioctl which takes the RTNL.  It's
not hard to find this stuff in net/core/dev.c.

Please do VMware driver developers a favour and pay a little more
attention to such subtleties.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html