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]

Message-ID: <1367921617.4126.36.camel@weser.hi.pengutronix.de>
Date:	Tue, 07 May 2013 12:13:37 +0200
From:	Lucas Stach <l.stach@...gutronix.de>
To:	Frank Li <lznuaa@...il.com>
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

Am Dienstag, den 07.05.2013, 18:02 +0800 schrieb Frank Li:
> 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?
> 
See Documentation/stable_kernel_rules.txt

Basically for this patch just add an
Cc: <stable@...r.kernel.org> # 3.9
below the sign-off area, but don't actually send the patch there. It
will get cherry-picked just based on the tag.

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