On 02.05.2014 20:24, Alan Somers wrote:
Author: asomers
Date: Fri May 2 16:24:09 2014
New Revision: 265232
URL: http://svnweb.freebsd.org/changeset/base/265232
Log:
Fix a panic caused by doing "ifconfig -am" while a lagg is being destroyed.
The thread that is destroying the lagg has already set sc->sc_psc=NULL when
the "ifconfig -am" thread gets to lacp_req(). It tries to dereference
sc->sc_psc and panics. The solution is for lacp_req() to check the value of
sc->sc_psc. If NULL, harmlessly return an lacp_opreq structure full of
zeros. Full details in GNATS.
Sorry, it looks like I've missed -net@ discussion.
While this patch minimizes panics, they still can occur.
If one thread performs lagg_clone_destroy() -> lagg_lacp_detach() (1) ->
lacp_detach(), executing
struct lacp_softc *lsc = LACP_SOFTC(sc); (e.g. one line before sc_psc =
NULL assignment)
At the same moment, another thread (initiated by ifconfig) executes
struct lacp_softc *lsc = LACP_SOFTC(sc);
Then, thread #1 goes further to
LACP_LOCK_DESTROY(lsc);
free(lsc, M_DEVBUF);
After that, thread #2 performs
bzero(req, sizeof(struct lacp_opreq));
passes lsc check for NULL and crashes on destroyed mutex.
Briefly looking, we can remove WUNLOCK() before lacp_detach() in
lagg_lacp_detach() (I'm unsure about the reasons why do we need unlock
here).
lacp_req() is already protected by at least LAGG_RLOCK() so we should
get consistent view of sc->sc_psc.
PR: kern/189003
Reviewed by: timeout on freebsd-net@
MFC after: 3 weeks
Sponsored by: Spectra Logic Corporation
Modified:
head/sys/net/ieee8023ad_lacp.c
Modified: head/sys/net/ieee8023ad_lacp.c
==============================================================================
--- head/sys/net/ieee8023ad_lacp.c Fri May 2 16:15:34 2014
(r265231)
+++ head/sys/net/ieee8023ad_lacp.c Fri May 2 16:24:09 2014
(r265232)
@@ -590,10 +590,20 @@ lacp_req(struct lagg_softc *sc, caddr_t
{
struct lacp_opreq *req = (struct lacp_opreq *)data;
struct lacp_softc *lsc = LACP_SOFTC(sc);
- struct lacp_aggregator *la = lsc->lsc_active_aggregator;
+ struct lacp_aggregator *la;
- LACP_LOCK(lsc);
bzero(req, sizeof(struct lacp_opreq));
+
+ /*
+ * If the LACP softc is NULL, return with the opreq structure full of
+ * zeros. It is normal for the softc to be NULL while the lagg is
+ * being destroyed.
+ */
+ if (NULL == lsc)
+ return;
+
+ la = lsc->lsc_active_aggregator;
+ LACP_LOCK(lsc);
if (la != NULL) {
req->actor_prio = ntohs(la->la_actor.lip_systemid.lsi_prio);
memcpy(&req->actor_mac, &la->la_actor.lip_systemid.lsi_mac,
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
