linux-kernel - Re: [Intel-wired-lan] [PATCH] e1000e: prevent division by zero if TIMINCA is zero

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:	Tue, 10 May 2016 22:59:47 -0500
From:	Mark D Rustad <mrustad@...il.com>
To:	Jarod Wilson <jarod@...hat.com>
Cc:	Mark Rustad <mark.d.rustad@...el.com>,
	Denys Vlasenko <dvlasenk@...hat.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [Intel-wired-lan] [PATCH] e1000e: prevent division by zero if TIMINCA is zero

Jarod Wilson <jarod@...hat.com> wrote:

> On Fri, May 06, 2016 at 11:43:17PM +0000, Rustad, Mark D wrote:
>> Denys Vlasenko <dvlasenk@...hat.com> wrote:
>>
>>> Users report that under VMWare, er32(TIMINCA) returns zero.
>>> This causes division by zero at init time as follows:
>>>
>>> ==>            incvalue = er32(TIMINCA) & E1000_TIMINCA_INCVALUE_MASK;
>>>                for (i = 0; i < E1000_MAX_82574_SYSTIM_REREADS; i++) {
>>>                        /* latch SYSTIMH on read of SYSTIML */
>>>                        systim_next = (cycle_t)er32(SYSTIML);
>>>                        systim_next |= (cycle_t)er32(SYSTIMH) << 32;
>>>
>>>                        time_delta = systim_next - systim;
>>>                        temp = time_delta;
>>> ====>                  rem = do_div(temp, incvalue);
>>>
>>> This change makes kernel survive this, and users report that
>>> NIC does work after this change.
>>>
>>> Since on real hardware incvalue is never zero, this should not affect
>>> real hardware use case.
> ...
>> I seem to recall that this was rejected before because it really is  
>> VMWare's
>> bug and, if they fix it, any existing VMs that use this will just work.
>> Changing the driver will only fix it for vms that install a new driver. I
>> don't object to doing it, it just seems like not the most effective  
>> place to
>> address the issue.
>
> You could also have people who never update VMWare, for whom a kernel
> work-around would be better. I think it'd be best to address it both at
> the driver level and the emulated hardware level, to improve things for
> the most possible users. Those who update neither hypervisor or
> kernel/driver, well, they reap what they sow.

That is a sound argument for doing both. I would expect that there are more  
frozen VM images than host environments, but I can certainly imagine that  
some choose to freeze their host. Of course if everything is frozen there  
is no point at all. :-)

I am on an extended vacation, and don't work on e1000e anyway, so I will  
quit my kibitzing here.

--
Mark Rustad, MRustad@...il.com

Download attachment "signature.asc" of type "application/pgp-signature" (842 bytes)