From owner-svn-src-all@freebsd.org Mon Jul 6 14:52:10 2020 Return-Path: Delivered-To: svn-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 95245369DA5; Mon, 6 Jul 2020 14:52:10 +0000 (UTC) (envelope-from markj@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4B0pSV3QGlz48sh; Mon, 6 Jul 2020 14:52:10 +0000 (UTC) (envelope-from markj@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 599871FDB0; Mon, 6 Jul 2020 14:52:10 +0000 (UTC) (envelope-from markj@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 066EqAC7025537; Mon, 6 Jul 2020 14:52:10 GMT (envelope-from markj@FreeBSD.org) Received: (from markj@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 066EqAap025536; Mon, 6 Jul 2020 14:52:10 GMT (envelope-from markj@FreeBSD.org) Message-Id: <202007061452.066EqAap025536@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: markj set sender to markj@FreeBSD.org using -f From: Mark Johnston Date: Mon, 6 Jul 2020 14:52:10 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r362962 - head/sys/net X-SVN-Group: head X-SVN-Commit-Author: markj X-SVN-Commit-Paths: head/sys/net X-SVN-Commit-Revision: 362962 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 06 Jul 2020 14:52:10 -0000 Author: markj Date: Mon Jul 6 14:52:09 2020 New Revision: 362962 URL: https://svnweb.freebsd.org/changeset/base/362962 Log: iflib: Fix handling of mbuf cluster allocation failures. When refilling an rx freelist, make sure we only update the hardware producer index if at least one cluster was allocated. Otherwise the NIC is programmed to write a previously used cluster, typically resulting in a use-after-free when packet data is written by the hardware. Also make sure that we don't update the fragment index cursor if the last allocation attempt didn't succeed. For at least Intel drivers, iflib assumes that the consumer index and fragment index cursor stay in lockstep, but this assumption was violated in the face of cluster allocation failures. Reported and tested by: pho Reviewed by: gallatin, hselasky MFC after: 2 weeks Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D25489 Modified: head/sys/net/iflib.c Modified: head/sys/net/iflib.c ============================================================================== --- head/sys/net/iflib.c Mon Jul 6 14:00:20 2020 (r362961) +++ head/sys/net/iflib.c Mon Jul 6 14:52:09 2020 (r362962) @@ -1975,7 +1975,8 @@ _iflib_fl_refill(if_ctx_t ctx, iflib_fl_t fl, int coun 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) + cl = m_cljget(NULL, M_NOWAIT, fl->ifl_buf_size); + if (__predict_false(cl == NULL)) break; cb_arg.error = 0; @@ -1983,12 +1984,8 @@ _iflib_fl_refill(if_ctx_t ctx, iflib_fl_t fl, int coun err = bus_dmamap_load(fl->ifl_buf_tag, sd_map[frag_idx], cl, fl->ifl_buf_size, _rxq_refill_cb, &cb_arg, BUS_DMA_NOWAIT); - if (err != 0 || cb_arg.error) { - /* - * !zone_pack ? - */ - if (fl->ifl_zone == zone_pack) - uma_zfree(fl->ifl_zone, cl); + if (__predict_false(err != 0 || cb_arg.error)) { + uma_zfree(fl->ifl_zone, cl); break; } @@ -2004,9 +2001,9 @@ _iflib_fl_refill(if_ctx_t ctx, iflib_fl_t fl, int coun BUS_DMASYNC_PREREAD); if (sd_m[frag_idx] == NULL) { - if ((m = m_gethdr(M_NOWAIT, MT_NOINIT)) == NULL) { + m = m_gethdr(M_NOWAIT, MT_NOINIT); + if (__predict_false(m == NULL)) break; - } sd_m[frag_idx] = m; } bit_set(fl->ifl_rx_bitmap, frag_idx); @@ -2036,25 +2033,32 @@ _iflib_fl_refill(if_ctx_t ctx, iflib_fl_t fl, int coun } } - if (i) { - iru.iru_pidx = pidx; - iru.iru_count = i; - ctx->isc_rxd_refill(ctx->ifc_softc, &iru); - fl->ifl_pidx = idx; - fl->ifl_credits = credits; - } - DBG_COUNTER_INC(rxd_flush); - if (fl->ifl_pidx == 0) - pidx = fl->ifl_size - 1; - else - pidx = fl->ifl_pidx - 1; + if (n < count - 1) { + if (i != 0) { + iru.iru_pidx = pidx; + iru.iru_count = i; + ctx->isc_rxd_refill(ctx->ifc_softc, &iru); + fl->ifl_pidx = idx; + fl->ifl_credits = credits; + } + DBG_COUNTER_INC(rxd_flush); + if (fl->ifl_pidx == 0) + pidx = fl->ifl_size - 1; + else + pidx = fl->ifl_pidx - 1; - bus_dmamap_sync(fl->ifl_ifdi->idi_tag, fl->ifl_ifdi->idi_map, - BUS_DMASYNC_PREREAD | BUS_DMASYNC_PREWRITE); - ctx->isc_rxd_flush(ctx->ifc_softc, fl->ifl_rxq->ifr_id, fl->ifl_id, pidx); - fl->ifl_fragidx = frag_idx + 1; - if (fl->ifl_fragidx == fl->ifl_size) - fl->ifl_fragidx = 0; + bus_dmamap_sync(fl->ifl_ifdi->idi_tag, fl->ifl_ifdi->idi_map, + BUS_DMASYNC_PREREAD | BUS_DMASYNC_PREWRITE); + ctx->isc_rxd_flush(ctx->ifc_softc, fl->ifl_rxq->ifr_id, + fl->ifl_id, pidx); + if (__predict_true(bit_test(fl->ifl_rx_bitmap, frag_idx))) { + fl->ifl_fragidx = frag_idx + 1; + if (fl->ifl_fragidx == fl->ifl_size) + fl->ifl_fragidx = 0; + } else { + fl->ifl_fragidx = frag_idx; + } + } return (n == -1 ? 0 : IFLIB_RXEOF_EMPTY); }