Index | Thread | Search

From:
Dave Voutila <dv@sisu.io>
Subject:
Re: nvme: avoid use-after-free after shutdown
To:
Jonathan Matthew <jonathan@d14n.org>
Cc:
tech@openbsd.org
Date:
Wed, 11 Feb 2026 12:18:24 -0500

Download raw body.

Thread
Jonathan Matthew <jonathan@d14n.org> writes:

> A recent report on bugs@ showed the nvme interrupt handler crashing
> during reboot.  It's a bit weird that the nvme controller raises an
> interrupt after we shut it down, but we shouldn't crash if that
> happens.
>
> The problem here is that we delete the IO queue and free the memory
> used for it, but the interrupt handler unconditionally tries to process
> it.  This diff just skips the IO queue if we've deleted it.  The
> bug reporter has confirmed that this fixes it for them.
>
> ok?
>
>
> Index: nvme.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/ic/nvme.c,v
> diff -u -p -r1.126 nvme.c
> --- nvme.c	14 Jan 2026 01:07:57 -0000	1.126
> +++ nvme.c	4 Feb 2026 04:45:59 -0000
> @@ -557,6 +574,7 @@ nvme_shutdown(struct nvme_softc *sc)
>  		printf("%s: unable to delete q, disabling\n", DEVNAME(sc));
>  		goto disable;
>  	}
> +	sc->sc_q = NULL;

I think this part can still race with a rogue interrupt because sc_q
will be non-NULL for some period of time after nvme_q_delete(), no?

>
>  	cc = nvme_read4(sc, NVME_CC);
>  	CLR(cc, NVME_CC_SHN_MASK);
> @@ -1574,7 +1593,7 @@ nvme_intr(void *xsc)
>  	struct nvme_softc *sc = xsc;
>  	int rv = 0;
>
> -	if (nvme_q_complete(sc, sc->sc_q))
> +	if (sc->sc_q != NULL && nvme_q_complete(sc, sc->sc_q))
>  		rv = 1;
>  	if (nvme_q_complete(sc, sc->sc_admin_q))
>  		rv = 1;
>
>
>