linux-kernel - Re: [PATCH] e1000e: write protect ICHx NVM to prevent malicious write/erase

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: <alpine.LFD.2.00.0810011731350.3341@nehalem.linux-foundation.org>
Date:	Wed, 1 Oct 2008 17:42:02 -0700 (PDT)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Jesse Brandeburg <jesse.brandeburg@...el.com>
cc:	jeff@...zik.org, davem@...emloft.net, linux-kernel@...r.kernel.org,
	netdev@...r.kernel.org, arjan@...ux.intel.com,
	Bruce Allan <bruce.w.allan@...el.com>, arjan@...ux.intel.com
Subject: Re: [PATCH] e1000e: write protect ICHx NVM to prevent malicious
 write/erase

On Wed, 1 Oct 2008, Jesse Brandeburg wrote:
>
> From: Bruce Allan <bruce.w.allan@...el.com>
> 
> Set the hardware to ignore all write/erase cycles to the GbE region in
> the ICHx NVM.  This feature can be disabled by the WriteProtectNVM module
> parameter (enabled by default) only after a hardware reset, but
> the machine must be power cycled before trying to enable writes.

Thanks, applied.

One thing that I did notice when I looked at the driver is that I don't 
see any serialization what-so-ever around a lot of the special accesses.

There's all these different routines that do

	ret_val = e1000_acquire_swflag_ich8lan(hw);
	if (ret_val)
		return retval;
	...
	e1000_release_swflag_ich8lan(hw);

but as far as I can tell, there is absolutely _nothing_ that prevents 
these from being done concurrently by software. 

Yeah, yeah, I'm sure most of them end up being single-threaded and only 
called over the probe sequence (well, at least I _hope_ so), but it sure 
isn't obvious. People call e1000_read_nvm() from various different 
contexts, and I'm not seeing what - if anything - protects two concurrent 
ethtool queries, for example.

Imagine that you run ethtool concurrently (even on a UP machine with 
preemption of just a sleeping op), and tell me that having two 
e1000_acquire_swflag_ich8lan/e1000_release_swflag_ich8lan sequences nest 
(or overlap) works. I don't think it does.

That E1000_EXTCNF_CTRL_SWFLAG is _not_ a lock against other threads, it's 
purely a lock against the hardware itself. And maybe I'm missing some 
locking, but I can't see it.

Same goes for the PHY accesses etc afaik. Hmm?

		Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/