Module Name: src Committed By: mrg Date: Thu Oct 5 21:43:02 UTC 2023
Modified Files: src/sys/dev/pci: if_rge.c Log Message: rge(4): check for all errors in rx buffer allocation should fix a crash seen by by Chavdar Ivanov reported on current-users. move the rx and tx list clean up into their own functions, and call the rx clean up function from the init function if something fails. this should fix a potential leak in this case, and generally frees up memory that won't be used without a successful init phase again. slight application of 'static', much more could be done. To generate a diff of this commit: cvs rdiff -u -r1.25 -r1.26 src/sys/dev/pci/if_rge.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Modified files: Index: src/sys/dev/pci/if_rge.c diff -u src/sys/dev/pci/if_rge.c:1.25 src/sys/dev/pci/if_rge.c:1.26 --- src/sys/dev/pci/if_rge.c:1.25 Wed Dec 21 05:19:15 2022 +++ src/sys/dev/pci/if_rge.c Thu Oct 5 21:43:02 2023 @@ -1,4 +1,4 @@ -/* $NetBSD: if_rge.c,v 1.25 2022/12/21 05:19:15 nonaka Exp $ */ +/* $NetBSD: if_rge.c,v 1.26 2023/10/05 21:43:02 mrg Exp $ */ /* $OpenBSD: if_rge.c,v 1.9 2020/12/12 11:48:53 jan Exp $ */ /* @@ -18,7 +18,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: if_rge.c,v 1.25 2022/12/21 05:19:15 nonaka Exp $"); +__KERNEL_RCSID(0, "$NetBSD: if_rge.c,v 1.26 2023/10/05 21:43:02 mrg Exp $"); #include <sys/types.h> @@ -107,8 +107,10 @@ void rge_ifmedia_sts(struct ifnet *, st int rge_allocmem(struct rge_softc *); int rge_newbuf(struct rge_softc *, int); void rge_discard_rxbuf(struct rge_softc *, int); -int rge_rx_list_init(struct rge_softc *); -void rge_tx_list_init(struct rge_softc *); +static int rge_rx_list_init(struct rge_softc *); +static void rge_rx_list_fini(struct rge_softc *); +static void rge_tx_list_init(struct rge_softc *); +static void rge_tx_list_fini(struct rge_softc *); int rge_rxeof(struct rge_softc *); int rge_txeof(struct rge_softc *); void rge_reset(struct rge_softc *); @@ -650,7 +652,7 @@ rge_init(struct ifnet *ifp) { struct rge_softc *sc = ifp->if_softc; uint32_t val; - int i; + unsigned i; rge_stop(ifp, 0); @@ -661,11 +663,12 @@ rge_init(struct ifnet *ifp) RGE_WRITE_2(sc, RGE_RXMAXSIZE, RGE_JUMBO_FRAMELEN); /* Initialize RX descriptors list. */ - if (rge_rx_list_init(sc) == ENOBUFS) { + int error = rge_rx_list_init(sc); + if (error != 0) { device_printf(sc->sc_dev, "init failed: no memory for RX buffers\n"); rge_stop(ifp, 1); - return (ENOBUFS); + return error; } /* Initialize TX descriptors. */ @@ -836,7 +839,6 @@ void rge_stop(struct ifnet *ifp, int disable) { struct rge_softc *sc = ifp->if_softc; - int i; if (disable) { callout_halt(&sc->sc_timeout, NULL); @@ -870,25 +872,8 @@ rge_stop(struct ifnet *ifp, int disable) sc->rge_head = sc->rge_tail = NULL; } - /* Free the TX list buffers. */ - for (i = 0; i < RGE_TX_LIST_CNT; i++) { - if (sc->rge_ldata.rge_txq[i].txq_mbuf != NULL) { - bus_dmamap_unload(sc->sc_dmat, - sc->rge_ldata.rge_txq[i].txq_dmamap); - m_freem(sc->rge_ldata.rge_txq[i].txq_mbuf); - sc->rge_ldata.rge_txq[i].txq_mbuf = NULL; - } - } - - /* Free the RX list buffers. */ - for (i = 0; i < RGE_RX_LIST_CNT; i++) { - if (sc->rge_ldata.rge_rxq[i].rxq_mbuf != NULL) { - bus_dmamap_unload(sc->sc_dmat, - sc->rge_ldata.rge_rxq[i].rxq_dmamap); - m_freem(sc->rge_ldata.rge_rxq[i].rxq_mbuf); - sc->rge_ldata.rge_rxq[i].rxq_mbuf = NULL; - } - } + rge_tx_list_fini(sc); + rge_rx_list_fini(sc); } /* @@ -1172,17 +1157,19 @@ rge_discard_rxbuf(struct rge_softc *sc, BUS_DMASYNC_PREREAD | BUS_DMASYNC_PREWRITE); } -int +static int rge_rx_list_init(struct rge_softc *sc) { - int i; + unsigned i; memset(sc->rge_ldata.rge_rx_list, 0, RGE_RX_LIST_SZ); for (i = 0; i < RGE_RX_LIST_CNT; i++) { sc->rge_ldata.rge_rxq[i].rxq_mbuf = NULL; - if (rge_newbuf(sc, i) == ENOBUFS) + if (rge_newbuf(sc, i) != 0) { + rge_rx_list_fini(sc); return (ENOBUFS); + } } sc->rge_ldata.rge_rxq_prodidx = sc->rge_ldata.rge_rxq_considx = 0; @@ -1191,10 +1178,26 @@ rge_rx_list_init(struct rge_softc *sc) return (0); } -void +static void +rge_rx_list_fini(struct rge_softc *sc) +{ + unsigned i; + + /* Free the RX list buffers. */ + for (i = 0; i < RGE_RX_LIST_CNT; i++) { + if (sc->rge_ldata.rge_rxq[i].rxq_mbuf != NULL) { + bus_dmamap_unload(sc->sc_dmat, + sc->rge_ldata.rge_rxq[i].rxq_dmamap); + m_freem(sc->rge_ldata.rge_rxq[i].rxq_mbuf); + sc->rge_ldata.rge_rxq[i].rxq_mbuf = NULL; + } + } +} + +static void rge_tx_list_init(struct rge_softc *sc) { - int i; + unsigned i; memset(sc->rge_ldata.rge_tx_list, 0, RGE_TX_LIST_SZ); @@ -1208,6 +1211,22 @@ rge_tx_list_init(struct rge_softc *sc) sc->rge_ldata.rge_txq_prodidx = sc->rge_ldata.rge_txq_considx = 0; } +static void +rge_tx_list_fini(struct rge_softc *sc) +{ + unsigned i; + + /* Free the TX list buffers. */ + for (i = 0; i < RGE_TX_LIST_CNT; i++) { + if (sc->rge_ldata.rge_txq[i].txq_mbuf != NULL) { + bus_dmamap_unload(sc->sc_dmat, + sc->rge_ldata.rge_txq[i].txq_dmamap); + m_freem(sc->rge_ldata.rge_txq[i].txq_mbuf); + sc->rge_ldata.rge_txq[i].txq_mbuf = NULL; + } + } +} + int rge_rxeof(struct rge_softc *sc) { @@ -1268,7 +1287,7 @@ rge_rxeof(struct rge_softc *sc) * reload the current one. */ - if (rge_newbuf(sc, i) == ENOBUFS) { + if (rge_newbuf(sc, i) != 0) { if (sc->rge_head != NULL) { m_freem(sc->rge_head); sc->rge_head = sc->rge_tail = NULL;