To set up an interrupt handler for a PCI device, you need to do
something like this:
pci_intr_handle_t ih;
pci_intr_map(pa, &ih);
printf(": %s", pci_intr_string(pa->pa_pc, ih));
pci_intr_establish(pa->pa_pc, ih, ...);
On arm64, pci_intr_handle_t is a pointer to an opaque struct. The
pci_intr_map() function allocates that struct using malloc(9) and in
order not to leak the memory, pci_intr_establish() frees the struct
using free(9). That is all fine, except that some drivers do things
in a slightly different order:
pci_intr_handle_t ih;
pci_intr_map(pa, &ih);
pci_intr_establish(pa->pa_pc, ih, ...);
printf(": %s", pci_intr_string(pa->pa_pc, ih));
That means the call to pci_intr_string() is a use-after-free. The
consequences are rather mild; it prints the wrong string. But this
isn't right.
Now the big question is whether the arm64 implementation needs to be
changed, or whether we should fix the drivers. Below is a fix to
nvme(4) for example.
Thoughts?
Index: dev/pci/nvme_pci.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/nvme_pci.c,v
retrieving revision 1.7
diff -u -p -r1.7 nvme_pci.c
--- dev/pci/nvme_pci.c 10 Jan 2018 15:45:46 -0000 1.7
+++ dev/pci/nvme_pci.c 31 May 2019 20:33:46 -0000
@@ -90,6 +90,7 @@ nvme_pci_attach(struct device *parent, s
struct pci_attach_args *pa = aux;
pcireg_t maptype;
pci_intr_handle_t ih;
+ const char *intrstr;
int msi = 1;
psc->psc_pc = pa->pa_pc;
@@ -113,14 +114,18 @@ nvme_pci_attach(struct device *parent, s
msi = 0;
}
+ intrstr = pci_intr_string(pa->pa_pc, ih);
sc->sc_ih = pci_intr_establish(pa->pa_pc, ih, IPL_BIO,
msi ? nvme_intr : nvme_intr_intx, sc, DEVNAME(sc));
if (sc->sc_ih == NULL) {
- printf(": unable to establish interrupt\n");
+ printf(": unable to establish interrupt");
+ if (intrstr != NULL)
+ printf(" at %s", intrstr);
+ printf("\n");
goto unmap;
}
- printf(": %s", pci_intr_string(pa->pa_pc, ih));
+ printf(": %s", intrstr);
if (nvme_attach(sc) != 0) {
/* error printed by nvme_attach() */
goto disestablish;
