From 077b71e0fa3ad5840a2b555ba5e3a6a921112309 Mon Sep 17 00:00:00 2001 From: wpaul Date: Thu, 5 May 2005 06:14:59 +0000 Subject: [PATCH] Avoid sleeping with mutex held in kern_ndis.c. Remove unused fields from ndis_miniport_block. Fix a bug in KeFlushQueuedDpcs() (we weren't calculating the kq pointer correctly). In if_ndis.c, clear the IFF_RUNNING flag before calling ndis_halt_nic(). Add some guards in kern_ndis.c to avoid letting anyone invoke ndis_get_info() or ndis_set_info() if the NIC isn't fully initialized. Apparently, mdnsd will sometimes try to invoke the ndis_ioctl() routine at exactly the wrong moment (to futz with its multicast filters) when the interface comes up, and can trigger a crash unless we guard against it. --- sys/compat/ndis/kern_ndis.c | 57 ++++++++++++++++++--------------- sys/compat/ndis/ndis_var.h | 7 ---- sys/compat/ndis/subr_ntoskrnl.c | 3 +- sys/dev/if_ndis/if_ndis.c | 4 +-- 4 files changed, 34 insertions(+), 37 deletions(-) diff --git a/sys/compat/ndis/kern_ndis.c b/sys/compat/ndis/kern_ndis.c index 0dfcb53e32e..f45f3145527 100644 --- a/sys/compat/ndis/kern_ndis.c +++ b/sys/compat/ndis/kern_ndis.c @@ -838,8 +838,6 @@ ndis_set_info(arg, oid, buf, buflen) sc = arg; - mtx_lock(&ndis_req_mtx); - KeAcquireSpinLock(&sc->ndis_block->nmb_lock, &irql); if (sc->ndis_block->nmb_pendingreq != NULL) @@ -847,16 +845,20 @@ ndis_set_info(arg, oid, buf, buflen) else sc->ndis_block->nmb_pendingreq = (ndis_request *)sc; + NDIS_LOCK(sc); setfunc = sc->ndis_chars->nmc_setinfo_func; adapter = sc->ndis_block->nmb_miniportadapterctx; - if (adapter == NULL || setfunc == NULL) { + if (adapter == NULL || setfunc == NULL || + sc->ndis_block->nmb_devicectx == NULL) { sc->ndis_block->nmb_pendingreq = NULL; + NDIS_UNLOCK(sc); KeReleaseSpinLock(&sc->ndis_block->nmb_lock, irql); - mtx_unlock(&ndis_req_mtx); return(ENXIO); } + NDIS_UNLOCK(sc); + rval = MSCALL6(setfunc, adapter, oid, buf, *buflen, &byteswritten, &bytesneeded); @@ -865,13 +867,13 @@ ndis_set_info(arg, oid, buf, buflen) KeReleaseSpinLock(&sc->ndis_block->nmb_lock, irql); if (rval == NDIS_STATUS_PENDING) { + mtx_lock(&ndis_req_mtx); error = msleep(&sc->ndis_block->nmb_setstat, &ndis_req_mtx, curthread->td_priority|PDROP, "ndisset", 5 * hz); rval = sc->ndis_block->nmb_setstat; - } else - mtx_unlock(&ndis_req_mtx); + } if (byteswritten) @@ -1044,20 +1046,22 @@ ndis_reset_nic(arg) ndis_handle adapter; ndis_reset_handler resetfunc; uint8_t addressing_reset; - struct ifnet *ifp; int rval; uint8_t irql; sc = arg; - ifp = &sc->arpcom.ac_if; + NDIS_LOCK(sc); adapter = sc->ndis_block->nmb_miniportadapterctx; resetfunc = sc->ndis_chars->nmc_reset_func; - if (adapter == NULL || resetfunc == NULL) + if (adapter == NULL || resetfunc == NULL || + sc->ndis_block->nmb_devicectx == NULL) { + NDIS_UNLOCK(sc); return(EIO); + } - mtx_lock(&ndis_req_mtx); + NDIS_UNLOCK(sc); if (NDIS_SERIALIZED(sc->ndis_block)) KeAcquireSpinLock(&sc->ndis_block->nmb_lock, &irql); @@ -1068,10 +1072,10 @@ ndis_reset_nic(arg) KeReleaseSpinLock(&sc->ndis_block->nmb_lock, irql); if (rval == NDIS_STATUS_PENDING) { + mtx_lock(&ndis_req_mtx); msleep(sc, &ndis_req_mtx, curthread->td_priority|PDROP, "ndisrst", 0); - } else - mtx_unlock(&ndis_req_mtx); + } return(0); } @@ -1085,14 +1089,11 @@ ndis_halt_nic(arg) struct ndis_softc *sc; ndis_handle adapter; ndis_halt_handler haltfunc; - struct ifnet *ifp; #ifdef NDIS_REAP_TIMERS ndis_miniport_timer *t, *n; - uint8_t irql; #endif sc = arg; - ifp = &sc->arpcom.ac_if; NDIS_LOCK(sc); adapter = sc->ndis_block->nmb_miniportadapterctx; @@ -1101,6 +1102,8 @@ ndis_halt_nic(arg) return(EIO); } + sc->ndis_block->nmb_devicectx = NULL; + #ifdef NDIS_REAP_TIMERS /* * Drivers are sometimes very lax about cancelling all @@ -1110,7 +1113,6 @@ ndis_halt_nic(arg) * the timers reside will no longer be valid. */ - KeAcquireSpinLock(&sc->ndis_block->nmb_lock, &irql); t = sc->ndis_block->nmb_timerlist; while (t != NULL) { KeCancelTimer(&t->nmt_ktimer); @@ -1119,7 +1121,6 @@ ndis_halt_nic(arg) n->nmt_nexttimer = NULL; } sc->ndis_block->nmb_timerlist = NULL; - KeReleaseSpinLock(&sc->ndis_block->nmb_lock, irql); KeFlushQueuedDpcs(); #endif @@ -1132,9 +1133,7 @@ ndis_halt_nic(arg) haltfunc = sc->ndis_chars->nmc_halt_func; NDIS_UNLOCK(sc); - mtx_lock(&ndis_req_mtx); MSCALL1(haltfunc, adapter); - mtx_unlock(&ndis_req_mtx); NDIS_LOCK(sc); sc->ndis_block->nmb_miniportadapterctx = NULL; @@ -1194,10 +1193,8 @@ ndis_init_nic(arg) for (i = 0; i < NdisMediumMax; i++) mediumarray[i] = i; - mtx_lock(&ndis_req_mtx); status = MSCALL6(initfunc, &openstatus, &chosenmedium, mediumarray, NdisMediumMax, block, block); - mtx_unlock(&ndis_req_mtx); /* * If the init fails, blow away the other exported routines @@ -1223,6 +1220,10 @@ ndis_init_nic(arg) ndis_thsuspend(curthread->td_proc, NULL, hz); + NDIS_LOCK(sc); + sc->ndis_block->nmb_devicectx = sc; + NDIS_UNLOCK(sc); + return(0); } @@ -1340,7 +1341,7 @@ ndis_get_info(arg, oid, buf, buflen) uint8_t irql; sc = arg; - mtx_lock(&ndis_req_mtx); + KeAcquireSpinLock(&sc->ndis_block->nmb_lock, &irql); if (sc->ndis_block->nmb_pendingreq != NULL) @@ -1348,16 +1349,20 @@ ndis_get_info(arg, oid, buf, buflen) else sc->ndis_block->nmb_pendingreq = (ndis_request *)sc; + NDIS_LOCK(sc); queryfunc = sc->ndis_chars->nmc_queryinfo_func; adapter = sc->ndis_block->nmb_miniportadapterctx; - if (adapter == NULL || queryfunc == NULL) { + if (adapter == NULL || queryfunc == NULL || + sc->ndis_block->nmb_devicectx == NULL) { sc->ndis_block->nmb_pendingreq = NULL; + NDIS_UNLOCK(sc); KeReleaseSpinLock(&sc->ndis_block->nmb_lock, irql); - mtx_unlock(&ndis_req_mtx); return(ENXIO); } + NDIS_UNLOCK(sc); + rval = MSCALL6(queryfunc, adapter, oid, buf, *buflen, &byteswritten, &bytesneeded); @@ -1368,13 +1373,13 @@ ndis_get_info(arg, oid, buf, buflen) /* Wait for requests that block. */ if (rval == NDIS_STATUS_PENDING) { + mtx_lock(&ndis_req_mtx); error = msleep(&sc->ndis_block->nmb_getstat, &ndis_req_mtx, curthread->td_priority|PDROP, "ndisget", 5 * hz); rval = sc->ndis_block->nmb_getstat; - } else - mtx_unlock(&ndis_req_mtx); + } if (byteswritten) *buflen = byteswritten; diff --git a/sys/compat/ndis/ndis_var.h b/sys/compat/ndis/ndis_var.h index 22e614e8e82..6e81b9961e4 100644 --- a/sys/compat/ndis/ndis_var.h +++ b/sys/compat/ndis/ndis_var.h @@ -1476,19 +1476,12 @@ struct ndis_miniport_block { * End of windows-specific portion of miniport block. Everything * below is BSD-specific. */ - struct ifnet *nmb_ifp; uint8_t nmb_dummybuf[128]; - device_object nmb_devobj; ndis_config_parm nmb_replyparm; - int nmb_pciidx; - device_t nmb_dev; ndis_resource_list *nmb_rlist; ndis_status nmb_getstat; ndis_status nmb_setstat; - vm_offset_t nmb_img; ndis_miniport_timer *nmb_timerlist; - io_workitem *nmb_workitems[NUMBER_OF_SINGLE_WORK_ITEMS]; - int nmb_item_idx; TAILQ_ENTRY(ndis_miniport_block) link; }; diff --git a/sys/compat/ndis/subr_ntoskrnl.c b/sys/compat/ndis/subr_ntoskrnl.c index e2e3838f0f0..f7194c0a759 100644 --- a/sys/compat/ndis/subr_ntoskrnl.c +++ b/sys/compat/ndis/subr_ntoskrnl.c @@ -3108,9 +3108,8 @@ KeFlushQueuedDpcs(void) * for them to drain. */ - kq = kq_queues; for (i = 0; i < mp_ncpus; i++) { - kq += i; + kq = kq_queues + i; KeSetEvent(&kq->kq_proc, 0, FALSE); KeWaitForSingleObject((nt_dispatch_header *)&kq->kq_done, 0, 0, TRUE, NULL); diff --git a/sys/dev/if_ndis/if_ndis.c b/sys/dev/if_ndis/if_ndis.c index 072146cbba5..e09250ddc4d 100644 --- a/sys/dev/if_ndis/if_ndis.c +++ b/sys/dev/if_ndis/if_ndis.c @@ -2768,14 +2768,14 @@ ndis_stop(sc) ifp = &sc->arpcom.ac_if; untimeout(ndis_tick, sc, sc->ndis_stat_ch); - ndis_halt_nic(sc); - NDIS_LOCK(sc); ifp->if_timer = 0; sc->ndis_link = 0; ifp->if_flags &= ~(IFF_RUNNING | IFF_OACTIVE); NDIS_UNLOCK(sc); + ndis_halt_nic(sc); + return; } -- 2.45.2