Bug 282168 - vtnet(4): panic in if_attach_internal() when kldloading
Summary: vtnet(4): panic in if_attach_internal() when kldloading
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 15.0-CURRENT
Hardware: amd64 Any
: --- Affects Only Me
Assignee: Mark Johnston
URL:
Keywords: crash
: 275381 (view as bug list)
Depends on:
Blocks:
 
Reported: 2024-10-17 20:19 UTC by Lexi Winter
Modified: 2024-11-07 15:15 UTC (History)
3 users (show)

See Also:
zlei: mfc-stable14+
zlei: mfc-stable13+


Attachments
kernel config (121 bytes, text/plain)
2024-10-17 22:15 UTC, Lexi Winter
no flags Details
kernel config (1.94 KB, text/plain)
2024-10-17 22:16 UTC, Lexi Winter
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Lexi Winter freebsd_triage 2024-10-17 20:19:42 UTC
bhyve guest: FreeBSD 15.0-CURRENT #0 lf/main-n269061-b4479cf2b3b9: Wed Sep 18 20:43:02 BST 2024     srcmastr@daphne.eden.le-fay.org:/src/obj/src/freebsd/lf/main/amd64.amd64/sys/LFV6

bhyve host: FreeBSD 15.0-CURRENT #43 lf/main-n269060-ffd494371643: Tue Sep 17 05:59:59 BST 2024     srcmastr@daphne.eden.le-fay.org:/src/obj/src/freebsd/lf/main/amd64.amd64/sys/LF

'kldloat virtio_pci' causes a kernel panic when vtnet is loaded.

stack trace (i have a vmcore if needed):

root@lily:~ # kldload virtio_pci
virtio_pci0: <VirtIO PCI (legacy) Network adapter> port 0x2000-0x203f mem 0xc0000000-0xc0001fff irq 18 at device 5.0 on pci0
root@lily:~ # vtnet0: <VirtIO Networking Adapter> on virtio_pci0


Fatal trap 12: page fault while in kernel mode
cpuid = 1; apic id = 01
fault virtual address   = 0x28
fault code              = supervisor read data, page not present
instruction pointer     = 0x20:0xffffffff8051deb8
stack pointer           = 0x28:0xfffffe004b379810
frame pointer           = 0x28:0xfffffe004b379850
code segment            = base rx0, limit 0xfffff, type 0x1b
                        = DPL 0, pres 1, long 1, def32 0, gran 1
processor eflags        = interrupt enabled, resume, IOPL = 0
current process         = 701 (devctl)
rdi: fffff80001021300 rsi: fffff80001021300 rdx: 0000000000000001
rcx: 0000000000000001  r8: 0000000000000010  r9: fffffe0001e0a040
rax: 0000000000000000 rbx: fffff80001f02000 rbp: fffffe004b379850
r10: fffff800011b9800 r11: 9e99ff93939e0000 r12: fffff8000186a800
r13: fffff800018c0b30 r14: ffffffff8070e492 r15: 0000000000000000
trap number             = 12
panic: page fault
cpuid = 1
time = 1729195870
KDB: stack backtrace:
#0 0xffffffff8045ed4d at kdb_backtrace+0x5d
#1 0xffffffff80414bdf at vpanic+0x13f
#2 0xffffffff80414a93 at panic+0x43
#3 0xffffffff8069d39b at trap_fatal+0x40b
#4 0xffffffff8069d3e6 at trap_pfault+0x46
#5 0xffffffff806747d8 at calltrap+0x8
#6 0xffffffff8051cd8e at if_attach_internal+0x4e
#7 0xffffffff80527f6c at ether_ifattach+0x2c
#8 0xffffffff8181d6d1 at vtnet_setup_interface+0x391
#9 0xffffffff8181cb53 at vtnet_attach+0x1a63
#10 0xffffffff8044e96c at device_attach+0x3ac
#11 0xffffffff81812497 at vtpci_legacy_probe_and_attach_child+0x77
#12 0xffffffff8044c029 at devclass_driver_added+0x29
#13 0xffffffff8045453b at device_do_deferred_actions+0x3b
#14 0xffffffff80453e3f at devctl2_ioctl+0x20f
#15 0xffffffff803811ab at devfs_ioctl+0xcb
#16 0xffffffff804fb30e at vn_ioctl+0xce
#17 0xffffffff8038180e at devfs_ioctl_f+0x1e
Uptime: 14s
Dumping 113 out of 1002 MB:..15%..29%..43%..57%..71%..85%..99%
Dump complete
Automatic reboot in 15 seconds - press a key on the console to abort
Comment 1 Lexi Winter freebsd_triage 2024-10-17 20:20:35 UTC
forgot to mention, this works fine if virtio_pci.ko and if_vtnet.ko are loaded prior to boot in loader.conf.  it's only kldload that triggers the problem.
Comment 2 Mark Johnston freebsd_committer freebsd_triage 2024-10-17 22:10:40 UTC
What is in your kernel config?
Comment 3 Lexi Winter freebsd_triage 2024-10-17 22:15:44 UTC
Created attachment 254318 [details]
kernel config
Comment 4 Lexi Winter freebsd_triage 2024-10-17 22:16:03 UTC
Created attachment 254319 [details]
kernel config
Comment 5 Lexi Winter freebsd_triage 2024-10-17 22:16:29 UTC
kernel config attached, note it's two files since "LFV6" includes "LF".  "LFV6" is the running kernel on this system.
Comment 6 Mark Johnston freebsd_committer freebsd_triage 2024-10-18 13:25:43 UTC
This is due to a VNET bug.  When a network driver attaches, it needs to have the current VNET set, e.g., because if_attach_internal()->if_addgroup() accesses the VNET-global variable V_ifg_head.  Normally this happens in device_probe_and_attach(), which is how drivers get attached if they're compiled into the kernel, but we're taking a different path here.

I posted a patch here which fixes the problem for me: https://reviews.freebsd.org/D47174
Comment 7 Lexi Winter freebsd_triage 2024-10-18 13:47:48 UTC
thanks, i'm in the process of doing a src+ports build now but once that's done i'll test this patch and see if it fixes the panic for me.
Comment 8 Lexi Winter freebsd_triage 2024-10-18 16:22:46 UTC
i can't build world with the patch from D47174 on top of fa573868f187956b384722a90392866769f4965a.

--- subr_bus.o ---
/data/build/src/freebsd/lf/main/sys/kern/subr_bus.c:2577:26: error: incomplete definition of type 'struct prison'
 2577 |         KASSERT(IS_DEFAULT_VNET(TD_TO_VNET(curthread)),
      |         ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~
 2578 |             ("device_attach: curthread is not in default vnet"));
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/data/build/src/freebsd/lf/main/sys/net/vnet.h:248:25: note: expanded from macro 'TD_TO_VNET'
  248 | #define TD_TO_VNET(td)          CRED_TO_VNET((td)->td_ucred)
      |                                 ^
/data/build/src/freebsd/lf/main/sys/net/vnet.h:247:41: note: expanded from macro 'CRED_TO_VNET'
  247 | #define CRED_TO_VNET(cr)        (cr)->cr_prison->pr_vnet
      |                                                ^
/data/build/src/freebsd/lf/main/sys/net/vnet.h:245:32: note: expanded from macro 'IS_DEFAULT_VNET'
  245 | #define IS_DEFAULT_VNET(arg)    ((arg) == vnet0)
      |                                   ^
/data/build/src/freebsd/lf/main/sys/sys/kassert.h:144:24: note: expanded from macro 'KASSERT'
  144 |         if (__predict_false(!(exp)))                                    \
      |             ~~~~~~~~~~~~~~~~~~^~~~~
/data/build/src/freebsd/lf/main/sys/sys/cdefs.h:315:51: note: expanded from macro '__predict_false'
  315 | #define __predict_false(exp)    __builtin_expect((exp), 0)
      |                                                   ^~~
/data/build/src/freebsd/lf/main/sys/sys/cpuset.h:152:8: note: forward declaration of 'struct prison'
  152 | struct prison;
      |        ^
/data/build/src/freebsd/lf/main/sys/kern/subr_bus.c:2579:20: error: incomplete definition of type 'struct prison'
 2579 |         CURVNET_SET_QUIET(TD_TO_VNET(curthread));
      |         ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
/data/build/src/freebsd/lf/main/sys/net/vnet.h:248:25: note: expanded from macro 'TD_TO_VNET'
  248 | #define TD_TO_VNET(td)          CRED_TO_VNET((td)->td_ucred)
      |                                 ^
/data/build/src/freebsd/lf/main/sys/net/vnet.h:247:41: note: expanded from macro 'CRED_TO_VNET'
  247 | #define CRED_TO_VNET(cr)        (cr)->cr_prison->pr_vnet
      |                                                ^
/data/build/src/freebsd/lf/main/sys/net/vnet.h:221:15: note: expanded from macro 'CURVNET_SET_QUIET'
  221 |         VNET_ASSERT((arg) != NULL && (arg)->vnet_magic_n == VNET_MAGIC_N, \
      |         ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  222 |             ("CURVNET_SET at %s:%d %s() curvnet=%p vnet=%p",            \
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  223 |             __FILE__, __LINE__, __func__, curvnet, (arg)));             \
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/data/build/src/freebsd/lf/main/sys/net/vnet.h:184:8: note: expanded from macro 'VNET_ASSERT'
  184 |         if (!(exp))                                                     \
      |               ^~~
/data/build/src/freebsd/lf/main/sys/sys/cpuset.h:152:8: note: forward declaration of 'struct prison'
  152 | struct prison;
      |        ^
/data/build/src/freebsd/lf/main/sys/kern/subr_bus.c:2579:20: error: incomplete definition of type 'struct prison'
 2579 |         CURVNET_SET_QUIET(TD_TO_VNET(curthread));
      |         ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
/data/build/src/freebsd/lf/main/sys/net/vnet.h:248:25: note: expanded from macro 'TD_TO_VNET'
  248 | #define TD_TO_VNET(td)          CRED_TO_VNET((td)->td_ucred)
      |                                 ^
/data/build/src/freebsd/lf/main/sys/net/vnet.h:247:41: note: expanded from macro 'CRED_TO_VNET'
  247 | #define CRED_TO_VNET(cr)        (cr)->cr_prison->pr_vnet
      |                                                ^
/data/build/src/freebsd/lf/main/sys/net/vnet.h:221:32: note: expanded from macro 'CURVNET_SET_QUIET'
  221 |         VNET_ASSERT((arg) != NULL && (arg)->vnet_magic_n == VNET_MAGIC_N, \
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  222 |             ("CURVNET_SET at %s:%d %s() curvnet=%p vnet=%p",            \
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  223 |             __FILE__, __LINE__, __func__, curvnet, (arg)));             \
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/data/build/src/freebsd/lf/main/sys/net/vnet.h:184:8: note: expanded from macro 'VNET_ASSERT'
  184 |         if (!(exp))                                                     \
      |               ^~~
/data/build/src/freebsd/lf/main/sys/sys/cpuset.h:152:8: note: forward declaration of 'struct prison'
  152 | struct prison;
      |        ^
/data/build/src/freebsd/lf/main/sys/kern/subr_bus.c:2579:20: error: incomplete definition of type 'struct prison'
 2579 |         CURVNET_SET_QUIET(TD_TO_VNET(curthread));
      |         ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
/data/build/src/freebsd/lf/main/sys/net/vnet.h:248:25: note: expanded from macro 'TD_TO_VNET'
  248 | #define TD_TO_VNET(td)          CRED_TO_VNET((td)->td_ucred)
      |                                 ^
/data/build/src/freebsd/lf/main/sys/net/vnet.h:247:41: note: expanded from macro 'CRED_TO_VNET'
  247 | #define CRED_TO_VNET(cr)        (cr)->cr_prison->pr_vnet
      |                                                ^
/data/build/src/freebsd/lf/main/sys/net/vnet.h:223:46: note: expanded from macro 'CURVNET_SET_QUIET'
  221 |         VNET_ASSERT((arg) != NULL && (arg)->vnet_magic_n == VNET_MAGIC_N, \
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  222 |             ("CURVNET_SET at %s:%d %s() curvnet=%p vnet=%p",            \
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  223 |             __FILE__, __LINE__, __func__, curvnet, (arg)));             \
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~
/data/build/src/freebsd/lf/main/sys/net/vnet.h:185:9: note: expanded from macro 'VNET_ASSERT'
  185 |                 panic msg;                                              \
      |                       ^~~
/data/build/src/freebsd/lf/main/sys/sys/cpuset.h:152:8: note: forward declaration of 'struct prison'
  152 | struct prison;
      |        ^
/data/build/src/freebsd/lf/main/sys/kern/subr_bus.c:2579:20: error: incomplete definition of type 'struct prison'
 2579 |         CURVNET_SET_QUIET(TD_TO_VNET(curthread));
      |         ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
/data/build/src/freebsd/lf/main/sys/net/vnet.h:248:25: note: expanded from macro 'TD_TO_VNET'
  248 | #define TD_TO_VNET(td)          CRED_TO_VNET((td)->td_ucred)
      |                                 ^
/data/build/src/freebsd/lf/main/sys/net/vnet.h:247:41: note: expanded from macro 'CRED_TO_VNET'
  247 | #define CRED_TO_VNET(cr)        (cr)->cr_prison->pr_vnet
      |                                                ^
/data/build/src/freebsd/lf/main/sys/net/vnet.h:225:12: note: expanded from macro 'CURVNET_SET_QUIET'
  225 |         curvnet = arg;  
      |                   ^~~
/data/build/src/freebsd/lf/main/sys/sys/cpuset.h:152:8: note: forward declaration of 'struct prison'
  152 | struct prison;
      |        ^
5 errors generated.
*** [subr_bus.o] Error code 1

make[2]: stopped making "all" in /data/build/obj/freebsd/data/build/src/freebsd/lf/main/amd64.amd64/sys/GENERIC
Comment 9 Mark Johnston freebsd_committer freebsd_triage 2024-10-18 16:34:46 UTC
(In reply to Lexi Winter from comment #8)
Oops.  Fixed by the latest revision.
Comment 10 Mark Johnston freebsd_committer freebsd_triage 2024-10-19 12:42:30 UTC
*** Bug 275381 has been marked as a duplicate of this bug. ***
Comment 11 commit-hook freebsd_committer freebsd_triage 2024-10-19 13:05:05 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=f4e35c044c8988b7452cefbdcc417f5fd723e021

commit f4e35c044c8988b7452cefbdcc417f5fd723e021
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2024-10-19 13:03:56 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2024-10-19 13:03:56 +0000

    bus: Set the current VNET in device_attach()

    Some drivers, in particular anything which creates an ifnet during
    attach, need to have the current VNET set, as if_attach_internal() and
    its callees access VNET-global variables.

    device_probe_and_attach() handles this, but this is not the only way to
    arrive in DEVICE_ATTACH.  In particular, bus drivers may invoke
    device_attach() directly, as does devctl2's DEV_ENABLE ioctl handler.
    So, set the current VNET in device_attach() instead.

    I believe it is always safe to use vnet0, as devctl2 ioctls are not
    permitted within a jail.

    PR:             282168
    Reviewed by:    zlei, kevans, bz, imp, glebius
    MFC after:      1 week
    Differential Revision:  https://reviews.freebsd.org/D47174

 sys/kern/subr_bus.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)
Comment 12 Lexi Winter freebsd_triage 2024-10-19 18:00:20 UTC
thanks, tested latest main and the bug seems to be fixed now.
Comment 13 Mark Johnston freebsd_committer freebsd_triage 2024-10-26 13:01:00 UTC
Thanks for the report.
Comment 14 commit-hook freebsd_committer freebsd_triage 2024-10-26 13:01:02 UTC
A commit in branch stable/14 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=2e80ea70b98ef75f8bea9155944e6f093c0fa828

commit 2e80ea70b98ef75f8bea9155944e6f093c0fa828
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2024-10-19 13:03:56 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2024-10-26 12:58:50 +0000

    bus: Set the current VNET in device_attach()

    Some drivers, in particular anything which creates an ifnet during
    attach, need to have the current VNET set, as if_attach_internal() and
    its callees access VNET-global variables.

    device_probe_and_attach() handles this, but this is not the only way to
    arrive in DEVICE_ATTACH.  In particular, bus drivers may invoke
    device_attach() directly, as does devctl2's DEV_ENABLE ioctl handler.
    So, set the current VNET in device_attach() instead.

    I believe it is always safe to use vnet0, as devctl2 ioctls are not
    permitted within a jail.

    PR:             282168
    Reviewed by:    zlei, kevans, bz, imp, glebius
    MFC after:      1 week
    Differential Revision:  https://reviews.freebsd.org/D47174

    (cherry picked from commit f4e35c044c8988b7452cefbdcc417f5fd723e021)

 sys/kern/subr_bus.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)
Comment 15 Zhenlei Huang freebsd_committer freebsd_triage 2024-11-07 14:23:45 UTC
Hi Mark, do you have plan to MFC the fix to stable/13 ?  Actually stable/13 is also affected when I was diagnosing PR 275381 .
Comment 16 commit-hook freebsd_committer freebsd_triage 2024-11-07 14:29:04 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=48c59140bd7d82de26966bbc9723a49de95abd94

commit 48c59140bd7d82de26966bbc9723a49de95abd94
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2024-10-19 13:03:56 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2024-11-07 14:26:34 +0000

    bus: Set the current VNET in device_attach()

    Some drivers, in particular anything which creates an ifnet during
    attach, need to have the current VNET set, as if_attach_internal() and
    its callees access VNET-global variables.

    device_probe_and_attach() handles this, but this is not the only way to
    arrive in DEVICE_ATTACH.  In particular, bus drivers may invoke
    device_attach() directly, as does devctl2's DEV_ENABLE ioctl handler.
    So, set the current VNET in device_attach() instead.

    I believe it is always safe to use vnet0, as devctl2 ioctls are not
    permitted within a jail.

    PR:             282168
    Reviewed by:    zlei, kevans, bz, imp, glebius
    MFC after:      1 week
    Differential Revision:  https://reviews.freebsd.org/D47174

    (cherry picked from commit f4e35c044c8988b7452cefbdcc417f5fd723e021)
    (cherry picked from commit 2e80ea70b98ef75f8bea9155944e6f093c0fa828)

 sys/kern/subr_bus.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)
Comment 17 Mark Johnston freebsd_committer freebsd_triage 2024-11-07 14:29:59 UTC
(In reply to Zhenlei Huang from comment #15)
This required a bit more care when merging, but it's done now.
Comment 18 Zhenlei Huang freebsd_committer freebsd_triage 2024-11-07 15:15:10 UTC
(In reply to Mark Johnston from comment #17)
> This required a bit more care when merging, but it's done now.
Thanks !