netdev - Re: [PATCH v4 1/1 net] net: fec: fix kernel oops when plug/unplug cable many times

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, 7 May 2013 18:02:27 +0800
From:	Frank Li <lznuaa@...il.com>
To:	Lucas Stach <l.stach@...gutronix.de>
Cc:	Frank Li <Frank.Li@...escale.com>,
	Francois Romieu <romieu@...zoreil.com>,
	Robert Schwebel <r.schwebel@...gutronix.de>,
	David Miller <davem@...emloft.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	Fabio Estevam <festevam@...il.com>,
	Shawn Guo <shawn.guo@...aro.org>
Subject: Re: [PATCH v4 1/1 net] net: fec: fix kernel oops when plug/unplug
 cable many times

2013/5/7 Lucas Stach <l.stach@...gutronix.de>:
> Am Dienstag, den 07.05.2013, 11:43 +0800 schrieb Frank Li:
>> reproduce steps
>>  1. flood ping from other machine
>>       ping -f -s 41000 IP
>>  2. run below script
>>     while [ 1 ]; do ethtool -s eth0 autoneg off;
>>     sleep 3;ethtool -s eth0 autoneg on; sleep 4; done;
>>
>> You can see oops in one hour.
>>
>> The reason is fec_restart clear BD but NAPI may use it.
>> The solution is disable NAPI and stop xmit when reset BD.
>> disable NAPI may sleep, so fec_restart can't be call in
>> atomic context.
>>
>> Signed-off-by: Frank Li <Frank.Li@...escale.com>
> One minor nitpick below, otherwise
> Reviewed-by: Lucas Stach <l.stach@...gutronix.de>
> Tested-by: Lucas Stach <l.stach@...gutronix.de>
>
> Could this patch please be marked as a candidate for the 3.9 stable
> tree? It fixes a real and severe problem for me, as I seem to be able to
> trigger the bug much more easily than Frank.
>

How to mark as a candidate for the 3.9 stable?

>> ---
>> Change from v1 to v2
>>  * Add netif_tx_lock(ndev) to avoid xmit runing when reset hardware
>> Change from v2 to v3
>>  * Move put real statements after function variable declarations according to David's comments
>>  * Remove lock in adjust_link according to Lucas Stach's comments
>> Change from v3 to v4
>>  * rebase to latest net/master
>>  * remove hw_lock because not used again
>>  * reduce delay work to 0
>>  * group delay work related feild to one structure
>>  * call netif_device_detach() in fec_restart
>>
>>  drivers/net/ethernet/freescale/fec.h      |   10 ++++--
>>  drivers/net/ethernet/freescale/fec_main.c |   44 +++++++++++++++++++++-------
>>  2 files changed, 39 insertions(+), 15 deletions(-)
>>
> [...]
>>
>>  static void
>> @@ -644,8 +658,22 @@ fec_timeout(struct net_device *ndev)
>>
>>       ndev->stats.tx_errors++;
>>
>> -     fec_restart(ndev, fep->full_duplex);
>> -     netif_wake_queue(ndev);
>> +     fep->delay_work.timeout = 1;
> I would like to see a proper true/false used in conjunction with the
> bool data type.
>
>> +     schedule_delayed_work(&(fep->delay_work.delay_work), 0);
>> +}
>> +
>> +static void fec_enet_work(struct work_struct *work)
>> +{
>> +     struct fec_enet_private *fep =
>> +             container_of(work,
>> +                          struct fec_enet_private,
>> +                          delay_work.delay_work.work);
>> +
>> +     if (fep->delay_work.timeout) {
>> +             fep->delay_work.timeout = 0;
> Same as above.
>
>> +             fec_restart(fep->netdev, fep->full_duplex);
>> +             netif_wake_queue(fep->netdev);
>> +     }
>>  }
>>
> [...]
> --
> Pengutronix e.K.                           | Lucas Stach                 |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html