DF-5049 / fix.diff
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 | fix.diff — DF-5049: mfi system-PD lifetime: AEN PD removal (MR_EVT_PD_REMOVED) deletes the syspd child locklessly with no disable gate, and mfi_syspd_detach frees the softc under open descriptors and bios parked on the controller bioq. Part 1 (trigger side, sys/dev/raid/mfi/mfi.c): give the MR_EVT_PD_REMOVED handler the same lock discipline every other syspd-list walker has (mfi_config_lock + mfi_io_lock) and gate the delete through mfi_syspd_disable() so an open syspd is refused (EBUSY) instead of being freed under the open descriptor. Part 2 (sink side, sys/dev/raid/mfi/mfi_syspd.c): mfi_syspd_detach refuses to detach while the device is open (except during controller detach, which must make forward progress), marks the syspd DISABLED, and drains bios parked on the controller bioq that reference this softc, completing them with ENXIO. A DISABLED re-check in mfi_syspd_strategy makes racing bios error out instead of enqueueing against the dying softc. Residual (documented in VERDICT.md): a bio already built into an in-flight firmware command at the instant of delete can still complete through mfi_disk_complete after the softc free; fully closing that window needs in-flight command accounting (follow-up shared with the DF-5013 LD fix). --- a/sys/dev/raid/mfi/mfi.c 2026-09-26 11:29:12.879998603 +0000 +++ b/sys/dev/raid/mfi/mfi.c 2026-09-26 11:29:19.447913071 +0000 @@ -1515,20 +1515,32 @@ if (mfi_detect_jbod_change) { /* * If the removed device is a SYSPD then - * delete it + * delete it. DF-5049: serialize with the + * rest of the driver and gate the delete + * through mfi_syspd_disable() so an open + * syspd is refused (EBUSY) instead of being + * freed under the open descriptor. */ + lockmgr(&sc->mfi_config_lock, LK_EXCLUSIVE); + lockmgr(&sc->mfi_io_lock, LK_EXCLUSIVE); TAILQ_FOREACH(syspd, &sc->mfi_syspd_tqh, pd_link) { if (syspd->pd_id == - detail->args.pd.device_id) { - get_mplock(); - device_delete_child( - sc->mfi_dev, - syspd->pd_dev); - rel_mplock(); + detail->args.pd.device_id) break; - } } + if (syspd != NULL && + mfi_syspd_disable(syspd) == 0) { + lockmgr(&sc->mfi_io_lock, LK_RELEASE); + get_mplock(); + device_delete_child( + sc->mfi_dev, syspd->pd_dev); + rel_mplock(); + lockmgr(&sc->mfi_io_lock, + LK_EXCLUSIVE); + } + lockmgr(&sc->mfi_io_lock, LK_RELEASE); + lockmgr(&sc->mfi_config_lock, LK_RELEASE); } } if (detail->code == MR_EVT_PD_INSERTED) { --- a/sys/dev/raid/mfi/mfi_syspd.c 2026-09-26 11:29:12.879998603 +0000 +++ b/sys/dev/raid/mfi/mfi_syspd.c 2026-09-26 11:29:29.919776703 +0000 @@ -156,6 +156,9 @@ mfi_syspd_detach(device_t dev) { struct mfi_system_pd *sc; + struct mfi_softc *controller; + struct bio *bio; + int nbio, i; sc = device_get_softc(dev); lockmgr(&sc->pd_controller->mfi_io_lock, LK_EXCLUSIVE); @@ -165,6 +168,39 @@ lockmgr(&sc->pd_controller->mfi_io_lock, LK_RELEASE); return (EBUSY); } + /* + * DF-5049: never free the softc under an open descriptor unless + * the controller itself is detaching (shutdown must progress). + */ + if ((sc->pd_flags & MFI_DISK_FLAGS_OPEN) && + !sc->pd_controller->mfi_detaching) { + lockmgr(&sc->pd_controller->mfi_io_lock, LK_RELEASE); + return (EBUSY); + } + sc->pd_flags |= MFI_DISK_FLAGS_DISABLED; + + /* + * DF-5049: drain bios parked on the controller queue that still + * reference this softc via bio_driver_info; letting them be + * dequeued after the kfree below is a use-after-free + * (mfi_bio_command -> mfid->ld_flags, mfi_build_syspdio -> + * disk->pd_id). + */ + controller = sc->pd_controller; + nbio = controller->mfi_qstat[MFIQ_BIO].q_length; + for (i = 0; i < nbio; i++) { + bio = mfi_dequeue_bio(controller); + if (bio == NULL) + break; + if (bio->bio_driver_info == sc) { + bio->bio_buf->b_error = ENXIO; + bio->bio_buf->b_flags |= B_ERROR; + bio->bio_buf->b_resid = bio->bio_buf->b_bcount; + biodone(bio); + } else { + mfi_enqueue_bio(controller, bio); + } + } lockmgr(&sc->pd_controller->mfi_io_lock, LK_RELEASE); disk_destroy(&sc->pd_disk); @@ -277,6 +313,15 @@ bio->bio_driver_info = sc; lockmgr(&controller->mfi_io_lock, LK_EXCLUSIVE); + if (sc->pd_flags & MFI_DISK_FLAGS_DISABLED) { + /* DF-5049: syspd is going away */ + lockmgr(&controller->mfi_io_lock, LK_RELEASE); + bp->b_error = ENXIO; + bp->b_flags |= B_ERROR; + bp->b_resid = bp->b_bcount; + biodone(bio); + return (0); + } mfi_enqueue_bio(controller, bio); devstat_start_transaction(&sc->pd_devstat); mfi_startio(controller); |