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  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, 06 Oct 2015 17:50:29 -0400
From:	Jarod Wilson <>
To:	Alexander Duyck <>
Subject: Re: [Intel-wired-lan] [PATCH] igb: add more checks for disconnected

Alexander Duyck wrote:
> On 09/21/2015 09:14 PM, Jarod Wilson wrote:
>> Alexander Duyck wrote:
>>> On 09/21/2015 10:11 AM, Jarod Wilson wrote:
>>>> Some pci changes upcoming in 4.3 seem to cause additional disconnects,
>>>> which can happen at unfortuitous times for igb, leading to issues
>>>> such as
>>>> this, where the disconnect happened just before
>>>> igb_configure_tx_ring():
>>>> Note: this is a follow-up patch in addition to the previously submitted
>>>> "igb: don't unmap NULL hw_addr"
>>>> drivers/net/ethernet/intel/igb/igb_main.c | 15 +++++++++++++--
>>>> 1 file changed, 13 insertions(+), 2 deletions(-)
>>>> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c
>>>> b/drivers/net/ethernet/intel/igb/igb_main.c
>>>> index 6369f9e..7060edf 100644
>>>> --- a/drivers/net/ethernet/intel/igb/igb_main.c
>>>> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
>>>> @@ -952,6 +952,11 @@ static int igb_request_msix(struct igb_adapter
>>>> *adapter)
>>>> if (err)
>>>> goto err_out;
>>>> + if (E1000_REMOVED(hw->hw_addr)) {
>>>> + err = -EIO;
>>>> + goto err_free;
>>>> + }
>>>> +
>>>> for (i = 0; i < adapter->num_q_vectors; i++) {
>>>> struct igb_q_vector *q_vector = adapter->q_vector[i];
>>> Instead of using E1000_REMOVED we should just replace the
>>> adapter->hw.hw_addr in the setup of the itr_register with
>>> adapter->io_addr like you did for Rx/Tx below.
>> I just tried that, and it reliably blows up horrifically, wedging the
>> machine to the point where all I could get was a screen shot with my
>> phone thus far, when we jump from igb_request_msix() to
>> igb_configure_msix() to igb_assign_vector() and finally to
>> igb_write_ivar(), at least as best as I can tell from what I was able to
>> see in the trace remnants still on-screen.
> Take a look at array_rd32, that is buggy and doesn't match up with the
> rd32 implementation. If you fix that then the blow-up should go away.
> You shouldn't need to worry about itr_register since it will only get
> triggered if the interrupt can be enabled and that shouldn't be able to
> happen if the device is not present.
>> Just switching to adapter->io_addr everywhere seems to not work as noted
>> above. :\ Note that I'm also chasing this from the other end with the
>> author of the pci patches that seem to have triggered this, so the real
>> bug might be over in pci-land, but hardening against explosions in igb
>> still seems like a worthwhile effort here.
> I am pretty sure array_rd32 is the problem. If you fix that then I
> suspect you should quit seeing any further issues.

Okay, I've reworked array_rd32 a bit to call igb_rd32 as well, and with 
that and some other minor tweaks, nothing blowing up. The hardware still 
doesn't work when hotplugged, due to disappearing off the pci bus 
temporarily, but I think that's a pci hotplug issue, not igb's fault. 
Things seem to behave just fine if the device is connected at boot. 
Updated patch forthcoming for dissection...

Jarod Wilson

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at
Please read the FAQ at

Powered by blists - more mailing lists