netdev - RE: [PATCH] igb: reinit_locked() should be called with rtnl

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: <61CC2BC414934749BD9F5BF3D5D9404498748B57@ORSMSX112.amr.corp.intel.com>
Date:   Thu, 2 Jul 2020 20:05:31 +0000
From:   "Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>
To:     Francesco Ruggeri <fruggeri@...sta.com>,
        "Nguyen, Anthony L" <anthony.l.nguyen@...el.com>
CC:     Jakub Kicinski <kuba@...nel.org>,
        David Miller <davem@...emloft.net>,
        "open list" <linux-kernel@...r.kernel.org>,
        netdev <netdev@...r.kernel.org>,
        "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>
Subject: RE: [PATCH] igb: reinit_locked() should be called with rtnl_lock

> -----Original Message-----
> From: Francesco Ruggeri <fruggeri@...sta.com>
> Sent: Thursday, July 2, 2020 12:35
> To: Kirsher, Jeffrey T <jeffrey.t.kirsher@...el.com>
> Cc: Jakub Kicinski <kuba@...nel.org>; David Miller <davem@...emloft.net>;
> open list <linux-kernel@...r.kernel.org>; netdev <netdev@...r.kernel.org>;
> intel-wired-lan@...ts.osuosl.org
> Subject: Re: [PATCH] igb: reinit_locked() should be called with rtnl_lock
> 
> > Do not worry about the other Intel drivers, I have our developers looking at
> each of our drivers for the locking issue.
> >
> > @David Miller - I am picking up this patch
> 
> There seems to be a second race, independent from the original one, that
> results in a divide error:
> 
> kworker         reboot -f       tx packet
> 
> igb_reset_task
>                 __igb_shutdown
>                 rtnl_lock()
>                 ...
>                 igb_clear_interrupt_scheme
>                 igb_free_q_vectors
>                 adapter->num_tx_queues = 0
>                 ...
>                 rtnl_unlock()
> rtnl_lock()
> igb_reinit_locked
> igb_down
> igb_up
> netif_tx_start_all_queues
>                                 dev_hard_start_xmit
>                                 igb_xmit_frame
>                                 igb_tx_queue_mapping
>                                 Panics on
>                                 r_idx % adapter->num_tx_queues
> 
> Using in igb_reset_task a logic similar to the one in ixgbe_reset_subtask (bailing
> if __IGB_DOWN or __IGB_RESETTING is set) seems to avoid the panic.
> That logic was first introduced in ixgbe as part of commit 2f90b8657ec ('ixgbe:
> this patch adds support for DCB to the kernel and ixgbe driver').
> Both fixes seem to be needed.

So will you be sending a v2 of your patch to include the second fix?