'genet' driver starts interrupt handlers before ifp is allocated.
Version: git main @ d3f7975fcb346ea28dde079a9c04cff5ef20a8d7
gen_intr() uses sc->ifp here:
gen_attach() calls bus_setup_intr() here:
gen_attach() calls if_alloc() here:
gen_attach() could either hold GEN_LOCK() or complete the init before the bus_setup_intr() calls.
Thanks! Can you reproduce this? Do you know how?
I'm trying to reproduce this again, without much success. When I do, I'll post the steps and the core file.
No luck, with reproducing this.
Clearing the interrupts before enabling them would also prevent this race.
Created attachment 225600 [details]
I have attached a proposed patch. Simply reordering the blocks seems like it should work, and this version runs. Could you test it?
I tested the change (While sending iperf traffic rebooted the system) and it started fine. Although I couldn't reproduce the issue before the change either.
Regardless, this change will prevent an ifp de-reference.
I'm curious why the second bus_setup_intr() call was not also moved? I understand the gen_intr2 function does nothing currently, but it may have functionality added later that might also rely on ifp being set? Plus having both the "Install interrupt handlers" in the same place seems like a good idea to me.
Thanks for pointing out the inconsistency with the second interrupt; that was an oversight, fixed. Do you want to test that version, or should I just commit it?
Created attachment 225786 [details]
Just in case, I have attached the revised patch.
(In reply to Mike Karels from comment #7)
The second interrupt does nothing currently, but I tested it anyways. Works for me.
A commit in branch main references this bug:
Author: Mike Karels <karels@FreeBSD.org>
AuthorDate: 2021-06-20 16:10:26 +0000
Commit: Mike Karels <karels@FreeBSD.org>
CommitDate: 2021-06-20 16:17:13 +0000
genet: Fix potential crash during attach
As pointed out in the bug, the genet driver (RPi4 Ethernet) was
attaching the interrupts before the data structures were fully
initialized, causing a crash if an interrupt came in during the
attach. Fix by reordering code blocks.
Reported by: < ghuckriede at blackberry.com >
Reviewed by: < ghuckriede at blackberry.com > (informally)
MFC after: 3 days
sys/arm64/broadcom/genet/if_genet.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)