From nobody Tue Mar 26 09:56:29 2024 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4V3lZ55q2tz5FGd5; Tue, 26 Mar 2024 09:56:29 +0000 (UTC) (envelope-from git@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 "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4V3lZ55HmRz4JnV; Tue, 26 Mar 2024 09:56:29 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1711446989; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=O+ARetHKdSR/6RlnYxG5BWWyRQvGRIUFp5NsYmdYPVM=; b=RY2X2QGvRUEfojiYdIBgXMbVyq7TyjSsSSyk8NnjFIXD+VjKbEy7XHAM6LB3yf0zbmZjxK uoquG3/YkhoRakL731ABhWmhzHdU1GjfbFn2H+I5VlpmUI54lcqHBgjdlxHPfU7PWiRfNZ gQghZ2IDTGkL2MPEHMHWs8/glmnXN0ekBEXc+Yx1kwSYymqEuMMQqUVCMn1JhAoKWwofHC ma7rUoAl7USClGlHkvrZKGbTd2AjgtNQ0usRawjsDgyEzfnQRW2kP8HrQx6sW41SKD6iR9 2A17gse2owCGuuOn2c5M4zgCyI/ZTUjtKl5Vq+f6J21Nsx4VsiH2R+Wjoid+bQ== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1711446989; a=rsa-sha256; cv=none; b=kaHCoNwrk9J5x2pgoRWu08Lw/zVhB6rRORfoyPSjlbIWmDBDiihFH27hFCpis6POGpEWPO a5Wl1eQrsohJ9KEtOhCqJMduiAMtLgFf95KvEuRmDza6ILC2VgoVmnqOn17FnSWkpD5a4l 6cP30Pt/6qU1pPowQQSdHdWp1Juekk9Vo6rEypR9Pv6Df9hkSMdLJo1r6DeyKb7QX+NB+x EaZ+ZtNWg3WUm3pFCmZLPwA0vVT6S2NDN7rsZcfe5VuVREW+jgjKEtrKKpFRZ5qkefIc/e HotET9Ouz4K8qM80bJl2rQQvAHW5lL/JOnvp7l2Js5vTB47yS0AoPGGPzBZfZg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1711446989; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=O+ARetHKdSR/6RlnYxG5BWWyRQvGRIUFp5NsYmdYPVM=; b=Q/yurEHSFpSPcMwQ9oRNbBbfh2Y3lJKyqDxO0nuM3W7okMkR8gn8sp3407TUw6CiHZP1Sv R6wI+KyUtk1lKLqzJ3Og8onEa1MXelA3yUHSxk1/m5mHA0D3Vt2NA3PJtIMG1O3EodYNIJ Fp6/DEbwwmh+ncV2LzMP5DxVuS5Gk+0lnBixnxLj/tpEMO9Z3N0tkTsMDQx5WBYG/rgUPu G1SPgwr2A3nNfKzVlMK6Pdvy7Pb4t5wi9q9kJPl394c2VFUkJd2DKIDrkmT0VtN0Oce8/h GvN08zhHVLktZLkDqS0GZUYnXNE+tFlydSluRvULQhb3TKQZJroZyzZZTO5O3A== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (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 did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4V3lZ54tsvzw2P; Tue, 26 Mar 2024 09:56:29 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.17.1/8.17.1) with ESMTP id 42Q9uTiC009663; Tue, 26 Mar 2024 09:56:29 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 42Q9uTFG009661; Tue, 26 Mar 2024 09:56:29 GMT (envelope-from git) Date: Tue, 26 Mar 2024 09:56:29 GMT Message-Id: <202403260956.42Q9uTFG009661@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Tom Jones Subject: git: 73fdbfb91121 - main - netmap: Address errors on memory free in netmap_generic List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: thj X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 73fdbfb911215795c55c89870ebc5d9197bf2a23 Auto-Submitted: auto-generated The branch main has been updated by thj: URL: https://cgit.FreeBSD.org/src/commit/?id=73fdbfb911215795c55c89870ebc5d9197bf2a23 commit 73fdbfb911215795c55c89870ebc5d9197bf2a23 Author: Tom Jones AuthorDate: 2024-03-26 09:52:07 +0000 Commit: Tom Jones CommitDate: 2024-03-26 09:55:55 +0000 netmap: Address errors on memory free in netmap_generic netmap_generic keeps a pool of mbufs for handling transfers, these mbufs have an external buffer attached to them. If some cases other parts of the network stack can chain these mbufs, when this happens the normal pool destructor function can end up free'ing the pool mbufs twice: - A first time if a pool mbuf has been chained with another mbuf when its chain is freed - A second time when its entry in the pool is freed Additionally, if other parts of the stack demote a pool mbuf its interface reference will be cleared. In this case we deference a NULL pointer when trying to free the mbuf through the destructor. Store a reference to the adapter in ext_arg1 with the destructor callback so we can find the correct adapter when free'ing a pool mbuf. This change enables using netmap with epair interfaces. Reviewed By: vmaffione MFC after: 1 week Relnotes: yes Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D44371 --- sys/dev/netmap/netmap_generic.c | 29 ++++++++++++++++++++++------- sys/dev/netmap/netmap_kern.h | 4 +++- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/sys/dev/netmap/netmap_generic.c b/sys/dev/netmap/netmap_generic.c index 83210ae755c7..9b1218043a88 100644 --- a/sys/dev/netmap/netmap_generic.c +++ b/sys/dev/netmap/netmap_generic.c @@ -256,7 +256,7 @@ generic_netmap_unregister(struct netmap_adapter *na) * TX event is consumed. */ mtx_lock_spin(&kring->tx_event_lock); if (kring->tx_event) { - SET_MBUF_DESTRUCTOR(kring->tx_event, NULL); + SET_MBUF_DESTRUCTOR(kring->tx_event, NULL, NULL); } kring->tx_event = NULL; mtx_unlock_spin(&kring->tx_event_lock); @@ -271,16 +271,18 @@ generic_netmap_unregister(struct netmap_adapter *na) for_each_tx_kring(r, kring, na) { callout_drain(&kring->tx_event_callout); - mtx_destroy(&kring->tx_event_lock); + if (kring->tx_pool == NULL) { continue; } for (i=0; inum_tx_desc; i++) { if (kring->tx_pool[i]) { - m_freem(kring->tx_pool[i]); + m_free(kring->tx_pool[i]); + kring->tx_pool[i] = NULL; } } + mtx_destroy(&kring->tx_event_lock); nm_os_free(kring->tx_pool); kring->tx_pool = NULL; } @@ -434,7 +436,7 @@ out: static void generic_mbuf_dtor(struct mbuf *m) { - struct netmap_adapter *na = NA(GEN_TX_MBUF_IFP(m)); + struct netmap_adapter *na = GEN_TX_MBUF_NA(m); struct netmap_kring *kring; unsigned int r = MBUF_TXQ(m); unsigned int r_orig = r; @@ -458,6 +460,18 @@ generic_mbuf_dtor(struct mbuf *m) kring = na->tx_rings[r]; mtx_lock_spin(&kring->tx_event_lock); + + /* + * The netmap destructor can be called between us getting the + * reference and taking the lock, in that case the ring + * reference won't be valid. The destructor will free this mbuf + * so we can stop here. + */ + if (GEN_TX_MBUF_NA(m) == NULL) { + mtx_unlock_spin(&kring->tx_event_lock); + return; + } + if (kring->tx_event == m) { kring->tx_event = NULL; match = true; @@ -525,7 +539,7 @@ generic_netmap_tx_clean(struct netmap_kring *kring, int txqdisc) /* This mbuf has been dequeued but is still busy * (refcount is 2). * Leave it to the driver and replenish. */ - m_freem(m); + m_free(m); tx_pool[nm_i] = NULL; } @@ -638,7 +652,8 @@ generic_set_tx_event(struct netmap_kring *kring, u_int hwcur) return; } - SET_MBUF_DESTRUCTOR(m, generic_mbuf_dtor); + SET_MBUF_DESTRUCTOR(m, generic_mbuf_dtor, kring->na); + kring->tx_event = m; #ifdef __FreeBSD__ /* @@ -664,7 +679,7 @@ generic_set_tx_event(struct netmap_kring *kring, u_int hwcur) /* Decrement the refcount. This will free it if we lose the race * with the driver. */ - m_freem(m); + m_free(m); } /* diff --git a/sys/dev/netmap/netmap_kern.h b/sys/dev/netmap/netmap_kern.h index 8618aaf82299..dd736b46ae70 100644 --- a/sys/dev/netmap/netmap_kern.h +++ b/sys/dev/netmap/netmap_kern.h @@ -102,6 +102,7 @@ #define MBUF_TXQ(m) ((m)->m_pkthdr.flowid) #define MBUF_TRANSMIT(na, ifp, m) ((na)->if_transmit(ifp, m)) #define GEN_TX_MBUF_IFP(m) ((m)->m_pkthdr.rcvif) +#define GEN_TX_MBUF_NA(m) ((struct netmap_adapter *)(m)->m_ext.ext_arg1) #define NM_ATOMIC_T volatile int /* required by atomic/bitops.h */ /* atomic operations */ @@ -2395,9 +2396,10 @@ nm_generic_mbuf_dtor(struct mbuf *m) uma_zfree(zone_clust, m->m_ext.ext_buf); } -#define SET_MBUF_DESTRUCTOR(m, fn) do { \ +#define SET_MBUF_DESTRUCTOR(m, fn, na) do { \ (m)->m_ext.ext_free = (fn != NULL) ? \ (void *)fn : (void *)nm_generic_mbuf_dtor; \ + (m)->m_ext.ext_arg1 = na; \ } while (0) static inline struct mbuf *