Bug #11643
panic when detaching vioif
Start date:
Due date:
% Done:
100%
Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:
Description
While testing the latest illumos bits in a bhyve VM with 2GB of RAM and without a configured network interface the machine crashes shortly after boot when the vioif module is automatically unloaded.
debugging crash dump vmcore.0 (64-bit) from omniosce operating system: 5.11 omnios-master-7f6d3461785 (i86pc) image uuid: 912a587c-4d49-6f23-97c8-d9bc9b165ae5 panic message: BAD TRAP: type=e (#pf Page fault) rp=fffffe0004bfe2a0 addr=0 occurred in module "genunix" due to a NULL pointer dereference dump content: all kernel and user pages
ddi_regs_map_setup+0x91(fffffe0383d53d58, 0, fffffe0004bfe470, 0, 0, fffffe0004bfe46a) pci_config_setup+0x55(fffffe0383d53d58, fffffe0004bfe4c0) pci_intx_get_ispec+0x84(fffffe0383d5c800, fffffe0383d53d58, 1) pci_disable_intr+0x3c(fffffe0383d5c800, fffffe0383d53d58, fffffe0387547320, 1) pci_common_intr_ops+0x5b5(fffffe0383d5c800, fffffe0383d53d58, b, fffffe0387547320, 0) npe_intr_ops+0x13(fffffe0383d5c800, fffffe0383d53d58, b, fffffe0387547320, 0) i_ddi_intr_ops+0x51(fffffe0383d53d58, fffffe0383d53d58, b, fffffe0387547320, 0) ddi_intr_disable+0x5b(fffffe0387547320) virtio_interrupts_unwind+0x5c(fffffe0387547be0) virtio_interrupts_disable_locked+0x2c(fffffe0387547be0) virtio_interrupts_teardown+0x2c(fffffe0387547be0) virtio_fini+0x2b(fffffe0387547be0, 0) vioif_detach+0xb4(fffffe0383d53d58, 0) devi_detach+0x88(fffffe0383d53d58, 0) detach_node+0x49(fffffe0383d53d58, 8000000) i_ndi_unconfig_node+0xa9(fffffe0383d53d58, 4, 8000000) i_ddi_detachchild+0x33(fffffe0383d53d58, 8000000) devi_detach_node+0xd1(fffffe0383d53d58, 8000000) unconfig_immediate_children+0x176(fffffe0383d5c800, 0, 8000000, f7) devi_unconfig_common+0x1a1(fffffe0383d5c800, 0, 8000000, f7, 0) mt_config_thread+0x15f(fffffe03996380a0) thread_start+0xb()
This turns out to be due to memory corruption following a use-after-free. There were several other issues preventing a clean detach, not caught during the initial integration due to inadequate testing of the detach path:
- the
vif_macp
member of thevioif_t
is vestigial; during review, themac_register_t
was allocated and then freed entirely within the attach routine, but a subsequentmac_free()
of the now unset struct member was left in by mistake - a recursive mutex entry in the
virtio_fini()
routine, in thevirtio_device_reset()
function - if the consumer calls
virtio_shutdown()
, theVIRTIO_INITLEVEL_SHUTDOWN
flag is set invio_initlevel
; duringvirtio_fini()
we check to make sure all initlevel flags have been cleared -- butSHUTDOWN
does not represent anything that needs to be deinitialised and so must be ignored vioif
keepsvirtio_chain_t
objects around for each TX and RX buffer, freeing them invioif_free_bufs()
on detach; freeing a chain requires access to thevirtio_queue_t
to use the idspace for tracking descriptor slots, but that was already freed invirtio_fini()
-- the calls must be reversed to avoid use-after-free
Updated by Andy Fiddaman over 1 year ago
Final testing was effectively via modload /kernel/drv/amd64/vioif
and modunload -i `modinfo | grep vioif | awk '{print $1}'`
Updated by Electric Monk over 1 year ago
- Status changed from In Progress to Closed
- % Done changed from 0 to 100
git commit 89cb8ffb5df88f95defaae8f0f4f0c67ccd9d17e
commit 89cb8ffb5df88f95defaae8f0f4f0c67ccd9d17e Author: Andy Fiddaman <omnios@citrus-it.co.uk> Date: 2019-08-30T23:31:03.000Z 11643 panic when detaching vioif Reviewed by: Joshua M. Clulow <josh@sysmgr.org> Reviewed by: Toomas Soome <tsoome@me.com> Reviewed by: Robert Mustacchi <rm@joyent.com> Reviewed by: Rich Lowe <richlowe@richlowe.net> Approved by: Dan McDonald <danmcd@joyent.com>