Date: Wed, 30 Jan 2019 11:56:10 +0000 (UTC) From: Marius Strobl <marius@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-12@freebsd.org Subject: svn commit: r343575 - stable/12/sys/net Message-ID: <201901301156.x0UBuAbS084591@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: marius Date: Wed Jan 30 11:56:10 2019 New Revision: 343575 URL: https://svnweb.freebsd.org/changeset/base/343575 Log: MFC: r343481 - In _iflib_fl_refill(), don't mark an RX buffer as available in the corresponding bitmap before adding an mbuf has actually succeeded. Previously, m_gethdr(M_NOWAIT, ...) failing caused a "hole" in the RX ring but not in its bitmap. One implication of such a hole was that in a subsequent call to _iflib_fl_refill() with the RX buffer accounting still indicating another reclaimable buffer, bit_ffc(3) nevertheless returned -1 in frag_idx which in turn caused havoc when used as an index. Thus, additionally assert that frag_idx is 0 or greater. Another possible consequence of a hole in the RX ring was a NULL- dereference when trying to use the unallocated mbuf, for example in iflib_rxd_pkt_get(). This bug was introduced with r341095, MFCed to stable/12 in r343304. While at it, make the variable declarations in _iflib_fl_refill() conform to style(9) and remove redundant checks already performed by bit_ffc{,_at}(3). - In iflib_queues_alloc(), don't pass redundant M_ZERO to bit_alloc(3). Reported and tested by: pho Modified: stable/12/sys/net/iflib.c Directory Properties: stable/12/ (props changed) Modified: stable/12/sys/net/iflib.c ============================================================================== --- stable/12/sys/net/iflib.c Wed Jan 30 11:40:12 2019 (r343574) +++ stable/12/sys/net/iflib.c Wed Jan 30 11:56:10 2019 (r343575) @@ -1916,27 +1916,27 @@ _rxq_refill_cb(void *arg, bus_dma_segment_t *segs, int static void _iflib_fl_refill(if_ctx_t ctx, iflib_fl_t fl, int count) { + struct if_rxd_update iru; + struct rxq_refill_cb_arg cb_arg; struct mbuf *m; - int idx, frag_idx = fl->ifl_fragidx; - int pidx = fl->ifl_pidx; caddr_t cl, *sd_cl; struct mbuf **sd_m; - struct if_rxd_update iru; - struct rxq_refill_cb_arg cb_arg; bus_dmamap_t *sd_map; - int n, i = 0; bus_addr_t bus_addr, *sd_ba; - int err; + int err, frag_idx, i, idx, n, pidx; qidx_t credits; sd_m = fl->ifl_sds.ifsd_m; sd_map = fl->ifl_sds.ifsd_map; sd_cl = fl->ifl_sds.ifsd_cl; sd_ba = fl->ifl_sds.ifsd_ba; + pidx = fl->ifl_pidx; idx = pidx; + frag_idx = fl->ifl_fragidx; credits = fl->ifl_credits; - n = count; + i = 0; + n = count; MPASS(n > 0); MPASS(credits + n <= fl->ifl_size); @@ -1958,9 +1958,11 @@ _iflib_fl_refill(if_ctx_t ctx, iflib_fl_t fl, int coun * * If the cluster is still set then we know a minimum sized packet was received */ - bit_ffc_at(fl->ifl_rx_bitmap, frag_idx, fl->ifl_size, &frag_idx); - if ((frag_idx < 0) || (frag_idx >= fl->ifl_size)) - bit_ffc(fl->ifl_rx_bitmap, fl->ifl_size, &frag_idx); + bit_ffc_at(fl->ifl_rx_bitmap, frag_idx, fl->ifl_size, + &frag_idx); + if (frag_idx < 0) + bit_ffc(fl->ifl_rx_bitmap, fl->ifl_size, &frag_idx); + MPASS(frag_idx >= 0); if ((cl = sd_cl[frag_idx]) == NULL) { if ((cl = m_cljget(NULL, M_NOWAIT, fl->ifl_buf_size)) == NULL) break; @@ -1990,12 +1992,12 @@ _iflib_fl_refill(if_ctx_t ctx, iflib_fl_t fl, int coun bus_addr = sd_ba[frag_idx]; } - bit_set(fl->ifl_rx_bitmap, frag_idx); MPASS(sd_m[frag_idx] == NULL); if ((m = m_gethdr(M_NOWAIT, MT_NOINIT)) == NULL) { break; } sd_m[frag_idx] = m; + bit_set(fl->ifl_rx_bitmap, frag_idx); #if MEMORY_LOGGING fl->ifl_m_enqueued++; #endif @@ -2020,7 +2022,6 @@ _iflib_fl_refill(if_ctx_t ctx, iflib_fl_t fl, int coun fl->ifl_pidx = idx; fl->ifl_credits = credits; } - } if (i) { @@ -4890,7 +4891,6 @@ iflib_device_deregister(if_ctx_t ctx) for (j = 0, fl = rxq->ifr_fl; j < rxq->ifr_nfl; j++, fl++) free(fl->ifl_rx_bitmap, M_IFLIB); - } tqg = qgroup_if_config_tqg; if (ctx->ifc_admin_task.gt_uniq != NULL) @@ -5298,7 +5298,8 @@ iflib_queues_alloc(if_ctx_t ctx) } for (j = 0, fl = rxq->ifr_fl; j < rxq->ifr_nfl; j++, fl++) - fl->ifl_rx_bitmap = bit_alloc(fl->ifl_size, M_IFLIB, M_WAITOK|M_ZERO); + fl->ifl_rx_bitmap = bit_alloc(fl->ifl_size, M_IFLIB, + M_WAITOK); } /* TXQs */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201901301156.x0UBuAbS084591>