linux-kernel - Re: [PATCH] e1000 driver RX race condition fixed

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: <507C7D76.3010602@intel.com>
Date:	Mon, 15 Oct 2012 14:17:42 -0700
From:	Alexander Duyck <alexander.h.duyck@...el.com>
To:	Dmitry Fleytman <dmitry@...nix.com>
CC:	linux-kernel@...r.kernel.org, e1000-devel@...ts.sourceforge.net,
	Chris Webb <chris.webb@...stichosts.com>,
	Richard Davies <richard.davies@...stichosts.com>
Subject: Re: [PATCH] e1000 driver RX race condition fixed

The datasheets for most of the parts are available at:
http://developer.intel.com/products/ethernet/resource.htm

You just need to select one of the parts supported by e1000 and select
either the datasheet or software developers manual depending on the part.

Thanks,

Alex

On 10/15/2012 01:20 PM, Dmitry Fleytman wrote:
> Hello, Alex
>
> Many thanks for clarification. I think your assumption is
> correct and this is exactly what needs to be fixed in QEMU.
>
> Is there any publicly available specification for Intel devices that
> explains their operation in such a great details?
>
> Dmitry.
>
>
> On Mon, Oct 15, 2012 at 10:03 PM, Alexander Duyck
> <alexander.h.duyck@...el.com> wrote:
>> Hello Dmitry,
>>
>> My concern is that on many of our parts the behavior is to initialize
>> both the head and tail to 0, enable Rx for either the ring or device
>> depending on the queue configuration, and then allocate buffers and bump
>> tail to indicate that the new buffers are present.  The reason behind
>> enabling Rx and bumping tail is because that signals the DMA engine to
>> start fetching buffers.  In my experience most of our hardware will
>> ignore the tail bump if it is done first and the Rx is enabled.
>>
>> With both head and tail at the same value it should not be possible for
>> any of the devices to start a DMA.  This is probably what you should be
>> checking for in fixing QEMU/KVM as it may be incorrectly assuming it can
>> fetch the descriptor pointed to by tail.
>>
>> We have your patch in our queue and can test to verify my assumptions
>> are correct.  If they are we will let you know and reject the patch.
>>
>> Thanks,
>>
>> Alex
>>
>>
>>
>> On 10/15/2012 12:44 PM, Dmitry Fleytman wrote:
>>> Hello, Alex
>>>
>>> Originally this bug was reported for virtual machines running on top
>>> of QEMU/KVM.
>>> After patch preparation I've tested it on physical e1000 card and it
>>> worked fine.
>>>
>>> However, it could be I've missed something, as I see now other Intel
>>> drivers (e1000e, ixgb, etc.)
>>> use the same sequence (RX enable and then ring allocate), so I'm
>>> starting to suspect that this is
>>> the correct behavior.
>>>
>>> If you confirm this is the way HW works, the this patch should be
>>> ignored. This is pure QEMU bug and we'll fix it there.
>>>
>>> Thanks,
>>> Dmitry.
>>>
>>> On Mon, Oct 15, 2012 at 8:53 PM, Alexander Duyck
>>> <alexander.h.duyck@...el.com> wrote:
>>>> On 10/14/2012 10:19 AM, Dmitry Fleytman wrote:
>>>>> There is a race condition in e1000 driver.
>>>>> It enables HW receive before RX rings initalization.
>>>>> In case of specific timing this may lead to host memory corruption
>>>>> due to DMA write to arbitrary memory location.
>>>>> Following patch fixes this issue by reordering initialization steps.
>>>>>
>>>>> Other Intel network drivers does not seem to have this issue.
>>>>>
>>>>> Dmitry Fleytman (1):
>>>>>   RX initialization sequence fixed - enable RX after corresponding ring
>>>>>     initialization only
>>>>>
>>>>>  drivers/net/ethernet/intel/e1000/e1000_ethtool.c |    9 +++++----
>>>>>  drivers/net/ethernet/intel/e1000/e1000_main.c    |   18 ++++++++++++++++--
>>>>>  2 files changed, 21 insertions(+), 6 deletions(-)
>>>>>
>>>> What device was it you saw this issue with?  The reason why I ask is
>>>> because I suspect this change should cause most of our e1000 hardware to
>>>> lock up since normally if you allocate buffers and then enable Rx it
>>>> will mean the ring was not updated and it will treat it as if there are
>>>> no buffers available.
>>>>
>>>> Thanks,
>>>>
>>>> Alex
>
>

--
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/