Project

General

Profile

Bug #11643

panic when detaching vioif

Added by Andy Fiddaman about 1 year ago. Updated about 1 year ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
driver - device drivers
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 the vioif_t is vestigial; during review, the mac_register_t was allocated and then freed entirely within the attach routine, but a subsequent mac_free() of the now unset struct member was left in by mistake
  • a recursive mutex entry in the virtio_fini() routine, in the virtio_device_reset() function
  • if the consumer calls virtio_shutdown(), the VIRTIO_INITLEVEL_SHUTDOWN flag is set in vio_initlevel; during virtio_fini() we check to make sure all initlevel flags have been cleared -- but SHUTDOWN does not represent anything that needs to be deinitialised and so must be ignored
  • vioif keeps virtio_chain_t objects around for each TX and RX buffer, freeing them in vioif_free_bufs() on detach; freeing a chain requires access to the virtio_queue_t to use the idspace for tracking descriptor slots, but that was already freed in virtio_fini() -- the calls must be reversed to avoid use-after-free

History

#2

Updated by Joshua M. Clulow about 1 year ago

  • Description updated (diff)
#3

Updated by Andy Fiddaman about 1 year ago

  • Description updated (diff)
#4

Updated by Andy Fiddaman about 1 year ago

Final testing was effectively via modload /kernel/drv/amd64/vioif and modunload -i `modinfo | grep vioif | awk '{print $1}'`

#5

Updated by Electric Monk about 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>

Also available in: Atom PDF