Bug 282168
| Summary: | vtnet(4): panic in if_attach_internal() when kldloading | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | Base System | Reporter: | Lexi Winter <lexi> | ||||||
| Component: | kern | Assignee: | Mark Johnston <markj> | ||||||
| Status: | Closed FIXED | ||||||||
| Severity: | Affects Only Me | CC: | emaste, markj, zlei | ||||||
| Priority: | --- | Keywords: | crash | ||||||
| Version: | 15.0-CURRENT | Flags: | zlei:
mfc-stable14+
zlei: mfc-stable13+ |
||||||
| Hardware: | amd64 | ||||||||
| OS: | Any | ||||||||
| Attachments: |
|
||||||||
|
Description
Lexi Winter
2024-10-17 20:19:42 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. What is in your kernel config? Created attachment 254318 [details]
kernel config
Created attachment 254319 [details]
kernel config
kernel config attached, note it's two files since "LFV6" includes "LF". "LFV6" is the running kernel on this system. 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 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. 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
(In reply to Lexi Winter from comment #8) Oops. Fixed by the latest revision. *** Bug 275381 has been marked as a duplicate of this bug. *** 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(-) thanks, tested latest main and the bug seems to be fixed now. Thanks for the report. 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(-) 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 . 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(-) (In reply to Zhenlei Huang from comment #15) This required a bit more care when merging, but it's done now. (In reply to Mark Johnston from comment #17) > This required a bit more care when merging, but it's done now. Thanks ! |
