On Thu, Dec 08, 2011 at 17:46 +0100, Mike Belopuhov wrote:
> there are some m_freem's missing in the error code paths.
> i guess that's because freebsd code is a bit different
> and we didn't look closely at it.
> 
> ok?
> 
> @@ -2601,6 +2604,8 @@ no_split:
>       error = bus_dmamap_load_mbuf(rxr->rxdma.dma_tag, rxbuf->pmap,
>           mp, BUS_DMA_NOWAIT);
>       if (error) {
> +             if (mh)
> +                     m_freem(mh);
>               m_freem(mp);
>               return (error);
>       }

in fact, if we're failing here, mh is already dma-loaded,
so we have to unload it first and clear the m_head pointer.

also i forgot to note that split mbufs are not currently
used, so there's no change in the behavior and these bugs
are basically hidden.  nevertheless i think it makes sense
to fix them.

diff --git dev/pci/if_ix.c dev/pci/if_ix.c
index d3c268e..35ecb34 100644
--- dev/pci/if_ix.c
+++ dev/pci/if_ix.c
@@ -2546,7 +2546,7 @@ ixgbe_get_buf(struct rx_ring *rxr, int i)
 {
        struct ix_softc         *sc = rxr->sc;
        struct ixgbe_rx_buf     *rxbuf;
-       struct mbuf             *mh, *mp;
+       struct mbuf             *mp, *mh = NULL;
        int                     error;
        union ixgbe_adv_rx_desc *rxdesc;
        size_t                   dsize = sizeof(union ixgbe_adv_rx_desc);
@@ -2570,8 +2570,10 @@ ixgbe_get_buf(struct rx_ring *rxr, int i)
                goto no_split;
 
        mh = m_gethdr(M_DONTWAIT, MT_DATA);
-       if (mh == NULL)
+       if (mh == NULL) {
+               m_freem(mp);
                return (ENOBUFS);
+       }
 
        mh->m_pkthdr.len = mh->m_len = MHLEN;
        mh->m_len = MHLEN;
@@ -2581,6 +2583,7 @@ ixgbe_get_buf(struct rx_ring *rxr, int i)
        error = bus_dmamap_load_mbuf(rxr->rxdma.dma_tag, rxbuf->hmap,
            mh, BUS_DMA_NOWAIT);
        if (error) {
+               m_freem(mp);
                m_freem(mh);
                return (error);
        }
@@ -2600,6 +2603,11 @@ no_split:
        error = bus_dmamap_load_mbuf(rxr->rxdma.dma_tag, rxbuf->pmap,
            mp, BUS_DMA_NOWAIT);
        if (error) {
+               if (mh) {
+                       bus_dmamap_unload(rxr->rxdma.dma_tag, rxbuf->hmap);
+                       rxbuf->m_head = NULL;
+                       m_freem(mh);
+               }
                m_freem(mp);
                return (error);
        }

Reply via email to