Beagle-Bone-Black, 13-1 GENERICSD release image panics in PF: login: Fatal kernel mode data abort: 'Alignment Fault' on read trapframe: 0xd71a7930 FSR=00000001, FAR=c3328224, spsr=60000013 r0 =d71a7a48, r1 =27244242, r2 =bf798103, r3 =c3328224 r4 =d1494200, r5 =d14db000, r6 =27244265, r7 =00000000 r8 =c092c92c, r9 =d71a7b68, r10=00000001, r11=d71a79d8 r12=d7297918, ssp=d71a79c0, slr=d724a5e8, pc =d7282800 panic: Fatal abort cpuid = 0 time = 1659683836 KDB: stack backtrace: #0 0xc0355b70 at kdb_backtrace+0x48 #1 0xc02fcb50 at vpanic+0x170 #2 0xc02fc9e0 at vpanic+0 #3 0xc061bc14 at abort_align+0 #4 0xc061bc70 at abort_align+0x5c #5 0xc061b8e4 at abort_handler+0x480 #6 0xc05fac68 at exception_exit+0 #7 0xd7282800 at pf_syncookie_validate+0x3c #8 0xd724a5e8 at $a.110+0x20 #9 0xd726c868 at $a.166+0x34 #10 0xc0441f68 at pfil_run_hooks+0xa0 #11 0xc046e808 at ip_input+0x694 #12 0xc0440c70 at netisr_dispatch_src+0xf8 #13 0xc043863c at ether_demux+0x1a4 #14 0xc0439cd0 at ether_nh_input+0x480 #15 0xc0440c70 at netisr_dispatch_src+0xf8 #16 0xc0438b08 at ether_input+0x50 #17 0xc0cfbee4 at $a.6+0x5a0 pf.conf available on request
Can you correlate the faulting PC with the source line?
In my non-invariants copy of pf_syncookie_validate(), the instr at offset 0x3c is ldrexd r4, [r3], which loads a 64-bit value from the address in r3, which according to the register dump below is not 8-byte aligned. That's part of the atomic_add_64() implementation. But, on arm uint64_t has 8-byte alignment, so I don't quite see how the misalignment can arise.
The issue stems from VNET allocations only being aligned to 4. how about this: diff --git a/sys/net/vnet.c b/sys/net/vnet.c index 4f242b07f169..3a0d40a31b6a 100644 --- a/sys/net/vnet.c +++ b/sys/net/vnet.c @@ -377,7 +377,11 @@ vnet_data_alloc(int size) void *s; s = NULL; +#if __LP32__ + size = roundup2(size, 2 * sizeof(void *)); +#else size = roundup2(size, sizeof(void *)); +#endif sx_xlock(&vnet_data_free_lock); TAILQ_FOREACH(df, &vnet_data_free_head, vnd_link) { if (df->vnd_len < size) @@ -409,7 +413,11 @@ vnet_data_free(void *start_arg, int size) uintptr_t start; uintptr_t end; +#if __LP32__ + size = roundup2(size, 2 * sizeof(void *)); +#else size = roundup2(size, sizeof(void *)); +#endif start = (uintptr_t)start_arg; end = start + size; /*
heh, wow. there is no __LP32__ macro, someone(tm) will have to add one. In the meantime the point is to validate everything works fine after the size is rounded up to a multiply of 8.
This should do the trick: diff --git a/sys/net/vnet.c b/sys/net/vnet.c index 4f242b07f169..da1fd0f36785 100644 --- a/sys/net/vnet.c +++ b/sys/net/vnet.c @@ -377,7 +377,17 @@ vnet_data_alloc(int size) void *s; s = NULL; +#ifdef __ILP32__ + /* + * Note that the area allocated here might hold fields + * modified with 64-bit atomics. Accomodate it by rounding + * the size up to a multiple of 64-bit to ensure the + * expected alignment. + */ + size = roundup2(size, 2 * sizeof(void *)); +#else size = roundup2(size, sizeof(void *)); +#endif sx_xlock(&vnet_data_free_lock); TAILQ_FOREACH(df, &vnet_data_free_head, vnd_link) { if (df->vnd_len < size) @@ -409,7 +419,11 @@ vnet_data_free(void *start_arg, int size) uintptr_t start; uintptr_t end; +#ifdef __ILP32__ + size = roundup2(size, 2 * sizeof(void *)); +#else size = roundup2(size, sizeof(void *)); +#endif start = (uintptr_t)start_arg; end = start + size; /*
^Triage: this aging PR does not seem to be "In Progress".
(In reply to Mateusz Guzik from comment #5) Instead of an `ifdef`, you could use size = roundup2(size, max(sizeof(void *), sizeof(uint64_t)));
(In reply to Robert Clausecker from comment #7) Yeah max() is nicer than the #ifdef. Will you handle the change?
I'll try to get it done.
Maybe just make use of `__max_align_t`? This is exactly the use case it is intended for. That is `roundup2(size, _Alignof(__max_align_t)))`.
(In reply to John Baldwin from comment #10) > Maybe just make use of `__max_align_t`? It appears C11 want `max_align_t` to be defined in <stddef.h> [1]. The main and stable/15 branch has already require C17. So adding a new typedef `typedef __max_align_t max_align_t;` in sys/sys/stddef.h ? [1] https://en.cppreference.com/w/c/types/max_align_t.html
(In reply to Zhenlei Huang from comment #11) __max_align_t is in <sys/_types.h> in the kernel, and this is kernel code, so it should work fine. This variation on mjg's patch compiles for me: diff --git a/sys/net/vnet.c b/sys/net/vnet.c index a2b827052dd3..07bda6386b50 100644 --- a/sys/net/vnet.c +++ b/sys/net/vnet.c @@ -385,7 +385,7 @@ vnet_data_alloc(int size) void *s; s = NULL; - size = roundup2(size, sizeof(void *)); + size = roundup2(size, _Alignof(__max_align_t)); sx_xlock(&vnet_data_free_lock); TAILQ_FOREACH(df, &vnet_data_free_head, vnd_link) { if (df->vnd_len < size) @@ -417,7 +417,7 @@ vnet_data_free(void *start_arg, int size) uintptr_t start; uintptr_t end; - size = roundup2(size, sizeof(void *)); + size = roundup2(size, _Alignof(__max_align_t)); start = (uintptr_t)start_arg; end = start + size; /*
(In reply to John Baldwin from comment #10) > Maybe just make use of `__max_align_t`? This is exactly the use case it is intended > for. That is `roundup2(size, _Alignof(__max_align_t)))`. I think you're right. (In reply to Zhenlei Huang from comment #11) > So adding a new typedef `typedef __max_align_t max_align_t;` in sys/sys/stddef.h ? Ignore that. `max_align_t` has already been defined properly. ``` % grep 'max_align_t' /usr/include/stddef.h /usr/include/stddef.h:typedef __max_align_t max_align_t; ``` (In reply to John Baldwin from comment #12) > - size = roundup2(size, sizeof(void *)); > + size = roundup2(size, _Alignof(__max_align_t)); That looks good to me. Are you going to commit that ?
(In reply to Zhenlei Huang from comment #13) Oh, forgot to align the start of `modspace`. Try this, ``` --- a/sys/net/vnet.c +++ b/sys/net/vnet.c @@ -175,7 +175,7 @@ static MALLOC_DEFINE(M_VNET_DATA, "vnet_data", "VNET data"); * Space to store virtualized global variables from loadable kernel modules, * and the free list to manage it. */ -VNET_DEFINE_STATIC(char, modspace[VNET_MODMIN] __aligned(__alignof(void *))); +VNET_DEFINE_STATIC(char, modspace[VNET_MODMIN] __aligned(_Alignof(__max_align_t))); /* * A copy of the initial values of all virtualized global variables. @@ -385,7 +385,7 @@ vnet_data_alloc(int size) void *s; s = NULL; - size = roundup2(size, sizeof(void *)); + size = roundup2(size, _Alignof(__max_align_t)); sx_xlock(&vnet_data_free_lock); TAILQ_FOREACH(df, &vnet_data_free_head, vnd_link) { if (df->vnd_len < size) @@ -417,7 +417,7 @@ vnet_data_free(void *start_arg, int size) uintptr_t start; uintptr_t end; - size = roundup2(size, sizeof(void *)); + size = roundup2(size, _Alignof(__max_align_t)); start = (uintptr_t)start_arg; end = start + size; /* ```
Please feel free to commandeer this bug. I don't have time for it right now.
(In reply to Zhenlei Huang from comment #14) Yes, aligning modspace is needed as well. I'm happy if you commit that with my Reviewed by tag.
(In reply to John Baldwin from comment #16) Posted to https://reviews.freebsd.org/D55560 .
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=32beb3ae71cb320dbe4190a01c036943d99083b3 commit 32beb3ae71cb320dbe4190a01c036943d99083b3 Author: Zhenlei Huang <zlei@FreeBSD.org> AuthorDate: 2026-02-28 11:35:42 +0000 Commit: Zhenlei Huang <zlei@FreeBSD.org> CommitDate: 2026-02-28 11:35:42 +0000 vnet: Ensure the space allocated by vnet_data_alloc() is sufficent aligned Some 32-bit architectures, e.g., armv7, require strict 8-byte alignment while doing atomic 64-bit access. Hence aligning to the pointer type (4-byte alignment) does not meet the requirement on those architectures. Make the space allocated by vnet_data_alloc() sufficent aligned to avoid unaligned access. PR: 265639 Diagnosed by: markj Reviewed by: jhb, markj Co-authored-by: jhb MFC after: 5 days Differential Revision: https://reviews.freebsd.org/D55560 sys/net/vnet.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)
A commit in branch stable/15 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=973d607b284ba68e63f0386af44c28bfde15add2 commit 973d607b284ba68e63f0386af44c28bfde15add2 Author: Zhenlei Huang <zlei@FreeBSD.org> AuthorDate: 2026-02-28 11:35:42 +0000 Commit: Zhenlei Huang <zlei@FreeBSD.org> CommitDate: 2026-03-05 10:51:20 +0000 vnet: Ensure the space allocated by vnet_data_alloc() is sufficent aligned Some 32-bit architectures, e.g., armv7, require strict 8-byte alignment while doing atomic 64-bit access. Hence aligning to the pointer type (4-byte alignment) does not meet the requirement on those architectures. Make the space allocated by vnet_data_alloc() sufficent aligned to avoid unaligned access. PR: 265639 Diagnosed by: markj Reviewed by: jhb, markj Co-authored-by: jhb MFC after: 5 days Differential Revision: https://reviews.freebsd.org/D55560 (cherry picked from commit 32beb3ae71cb320dbe4190a01c036943d99083b3) sys/net/vnet.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)
A commit in branch stable/14 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=baee504b868b9417c815c0de6474a0d6e5d6b4ac commit baee504b868b9417c815c0de6474a0d6e5d6b4ac Author: Zhenlei Huang <zlei@FreeBSD.org> AuthorDate: 2026-02-28 11:35:42 +0000 Commit: Zhenlei Huang <zlei@FreeBSD.org> CommitDate: 2026-03-05 11:04:20 +0000 vnet: Ensure the space allocated by vnet_data_alloc() is sufficent aligned Some 32-bit architectures, e.g., armv7, require strict 8-byte alignment while doing atomic 64-bit access. Hence aligning to the pointer type (4-byte alignment) does not meet the requirement on those architectures. Make the space allocated by vnet_data_alloc() sufficent aligned to avoid unaligned access. PR: 265639 Diagnosed by: markj Reviewed by: jhb, markj Co-authored-by: jhb MFC after: 5 days Differential Revision: https://reviews.freebsd.org/D55560 (cherry picked from commit 32beb3ae71cb320dbe4190a01c036943d99083b3) (cherry picked from commit 973d607b284ba68e63f0386af44c28bfde15add2) sys/net/vnet.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=ec22c4022ddbc62be17b2bc65fe8b6113c8d76e7 commit ec22c4022ddbc62be17b2bc65fe8b6113c8d76e7 Author: Zhenlei Huang <zlei@FreeBSD.org> AuthorDate: 2026-02-28 11:35:42 +0000 Commit: Zhenlei Huang <zlei@FreeBSD.org> CommitDate: 2026-03-05 11:12:41 +0000 vnet: Ensure the space allocated by vnet_data_alloc() is sufficent aligned Some 32-bit architectures, e.g., armv7, require strict 8-byte alignment while doing atomic 64-bit access. Hence aligning to the pointer type (4-byte alignment) does not meet the requirement on those architectures. Make the space allocated by vnet_data_alloc() sufficent aligned to avoid unaligned access. PR: 265639 Diagnosed by: markj Reviewed by: jhb, markj Co-authored-by: jhb MFC after: 5 days Differential Revision: https://reviews.freebsd.org/D55560 (cherry picked from commit 32beb3ae71cb320dbe4190a01c036943d99083b3) (cherry picked from commit 973d607b284ba68e63f0386af44c28bfde15add2) (cherry picked from commit baee504b868b9417c815c0de6474a0d6e5d6b4ac) sys/net/vnet.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)
Fixed in main and supported stable branches.