From owner-svn-src-all@freebsd.org Fri May 3 20:38:44 2019 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 950681599F77; Fri, 3 May 2019 20:38:44 +0000 (UTC) (envelope-from rwatson@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) server-signature RSA-PSS (4096 bits) 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 38F42874A7; Fri, 3 May 2019 20:38:44 +0000 (UTC) (envelope-from rwatson@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 2011125E5F; Fri, 3 May 2019 20:38:44 +0000 (UTC) (envelope-from rwatson@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id x43Kch68065942; Fri, 3 May 2019 20:38:43 GMT (envelope-from rwatson@FreeBSD.org) Received: (from rwatson@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id x43KchLG065940; Fri, 3 May 2019 20:38:43 GMT (envelope-from rwatson@FreeBSD.org) Message-Id: <201905032038.x43KchLG065940@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: rwatson set sender to rwatson@FreeBSD.org using -f From: Robert Watson Date: Fri, 3 May 2019 20:38:43 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r347058 - head/sys/security/mac X-SVN-Group: head X-SVN-Commit-Author: rwatson X-SVN-Commit-Paths: head/sys/security/mac X-SVN-Commit-Revision: 347058 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 38F42874A7 X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-2.96 / 15.00]; local_wl_from(0.00)[FreeBSD.org]; NEURAL_HAM_MEDIUM(-1.00)[-0.999,0]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; NEURAL_HAM_SHORT(-0.96)[-0.958,0]; ASN(0.00)[asn:11403, ipnet:2610:1c1:1::/48, country:US] X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 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: Fri, 03 May 2019 20:38:44 -0000 Author: rwatson Date: Fri May 3 20:38:43 2019 New Revision: 347058 URL: https://svnweb.freebsd.org/changeset/base/347058 Log: When MAC is enabled and a policy module is loaded, don't unconditionally lock mac_ifnet_mtx, which protects labels on struct ifnet, unless at least one policy is actively using labels on ifnets. This avoids a global mutex acquire in certain fast paths -- most noticeably ifnet transmit. This was previously invisible by default, as no MAC policies were loaded by default, but recently became visible due to mac_ntpd being enabled by default. gallatin@ reports a reduction in PPS overhead from 300% to 2.2% with this change. We will want to explore further MAC Framework optimisation to reduce overhead further, but this brings things more back into the world of the sane. MFC after: 3 days Modified: head/sys/security/mac/mac_inet.c head/sys/security/mac/mac_internal.h head/sys/security/mac/mac_net.c Modified: head/sys/security/mac/mac_inet.c ============================================================================== --- head/sys/security/mac/mac_inet.c Fri May 3 20:05:31 2019 (r347057) +++ head/sys/security/mac/mac_inet.c Fri May 3 20:38:43 2019 (r347058) @@ -1,5 +1,5 @@ /*- - * Copyright (c) 1999-2002, 2007, 2009 Robert N. M. Watson + * Copyright (c) 1999-2002, 2007, 2009, 2019 Robert N. M. Watson * Copyright (c) 2001 Ilmar S. Habibulin * Copyright (c) 2001-2004 Networks Associates Technology, Inc. * Copyright (c) 2006 SPARTA, Inc. @@ -266,16 +266,17 @@ void mac_netinet_arp_send(struct ifnet *ifp, struct mbuf *m) { struct label *mlabel; + int locked; if (mac_policy_count == 0) return; mlabel = mac_mbuf_to_label(m); - MAC_IFNET_LOCK(ifp); + MAC_IFNET_LOCK(ifp, locked); MAC_POLICY_PERFORM_NOSLEEP(netinet_arp_send, ifp, ifp->if_label, m, mlabel); - MAC_IFNET_UNLOCK(ifp); + MAC_IFNET_UNLOCK(ifp, locked); } void @@ -310,16 +311,17 @@ void mac_netinet_igmp_send(struct ifnet *ifp, struct mbuf *m) { struct label *mlabel; + int locked; if (mac_policy_count == 0) return; mlabel = mac_mbuf_to_label(m); - MAC_IFNET_LOCK(ifp); + MAC_IFNET_LOCK(ifp, locked); MAC_POLICY_PERFORM_NOSLEEP(netinet_igmp_send, ifp, ifp->if_label, m, mlabel); - MAC_IFNET_UNLOCK(ifp); + MAC_IFNET_UNLOCK(ifp, locked); } void Modified: head/sys/security/mac/mac_internal.h ============================================================================== --- head/sys/security/mac/mac_internal.h Fri May 3 20:05:31 2019 (r347057) +++ head/sys/security/mac/mac_internal.h Fri May 3 20:38:43 2019 (r347058) @@ -1,5 +1,5 @@ /*- - * Copyright (c) 1999-2002, 2006, 2009 Robert N. M. Watson + * Copyright (c) 1999-2002, 2006, 2009, 2019 Robert N. M. Watson * Copyright (c) 2001 Ilmar S. Habibulin * Copyright (c) 2001-2004 Networks Associates Technology, Inc. * Copyright (c) 2006 nCircle Network Security, Inc. @@ -216,8 +216,24 @@ void mac_destroy_label(struct label *label); int mac_check_structmac_consistent(struct mac *mac); int mac_allocate_slot(void); -#define MAC_IFNET_LOCK(ifp) mtx_lock(&mac_ifnet_mtx) -#define MAC_IFNET_UNLOCK(ifp) mtx_unlock(&mac_ifnet_mtx) +/* + * Lock ifnets to protect labels only if ifnet labels are in use. + */ +#define MAC_IFNET_LOCK(ifp, locked) do { \ + if (mac_labeled & MPC_OBJECT_IFNET) { \ + mtx_lock(&mac_ifnet_mtx); \ + locked = 1; \ + } else { \ + locked = 0; \ + } \ +} while (0) + +#define MAC_IFNET_UNLOCK(ifp, locked) do { \ + if (locked) { \ + mtx_unlock(&mac_ifnet_mtx); \ + locked = 0; \ + } \ +} while (0) /* * MAC Framework per-object type functions. It's not yet clear how the Modified: head/sys/security/mac/mac_net.c ============================================================================== --- head/sys/security/mac/mac_net.c Fri May 3 20:05:31 2019 (r347057) +++ head/sys/security/mac/mac_net.c Fri May 3 20:38:43 2019 (r347058) @@ -1,5 +1,5 @@ /*- - * Copyright (c) 1999-2002, 2009 Robert N. M. Watson + * Copyright (c) 1999-2002, 2009, 2019 Robert N. M. Watson * Copyright (c) 2001 Ilmar S. Habibulin * Copyright (c) 2001-2004 Networks Associates Technology, Inc. * Copyright (c) 2006 SPARTA, Inc. @@ -77,6 +77,11 @@ __FBSDID("$FreeBSD$"); * XXXRW: struct ifnet locking is incomplete in the network code, so we use * our own global mutex for struct ifnet. Non-ideal, but should help in the * SMP environment. + * + * This lock is acquired only if a loaded policy is using ifnet labeling. + * This should not ever change during a MAC policy check, itself, but could + * change during setup/return from a check, so we have to condition unlock on + * previous lock. */ struct mtx mac_ifnet_mtx; MTX_SYSINIT(mac_ifnet_mtx, &mac_ifnet_mtx, "mac_ifnet", MTX_DEF); @@ -297,13 +302,14 @@ mac_ifnet_internalize_label(struct label *label, char void mac_ifnet_create(struct ifnet *ifp) { + int locked; if (mac_policy_count == 0) return; - MAC_IFNET_LOCK(ifp); + MAC_IFNET_LOCK(ifp, locked); MAC_POLICY_PERFORM_NOSLEEP(ifnet_create, ifp, ifp->if_label); - MAC_IFNET_UNLOCK(ifp); + MAC_IFNET_UNLOCK(ifp, locked); } void @@ -334,16 +340,17 @@ void mac_ifnet_create_mbuf(struct ifnet *ifp, struct mbuf *m) { struct label *label; + int locked; if (mac_policy_count == 0) return; label = mac_mbuf_to_label(m); - MAC_IFNET_LOCK(ifp); + MAC_IFNET_LOCK(ifp, locked); MAC_POLICY_PERFORM_NOSLEEP(ifnet_create_mbuf, ifp, ifp->if_label, m, label); - MAC_IFNET_UNLOCK(ifp); + MAC_IFNET_UNLOCK(ifp, locked); } MAC_CHECK_PROBE_DEFINE2(bpfdesc_check_receive, "struct bpf_d *", @@ -352,7 +359,7 @@ MAC_CHECK_PROBE_DEFINE2(bpfdesc_check_receive, "struct int mac_bpfdesc_check_receive(struct bpf_d *d, struct ifnet *ifp) { - int error; + int error, locked; /* Assume reader lock is enough. */ BPFD_LOCK_ASSERT(d); @@ -360,11 +367,11 @@ mac_bpfdesc_check_receive(struct bpf_d *d, struct ifne if (mac_policy_count == 0) return (0); - MAC_IFNET_LOCK(ifp); + MAC_IFNET_LOCK(ifp, locked); MAC_POLICY_CHECK_NOSLEEP(bpfdesc_check_receive, d, d->bd_label, ifp, ifp->if_label); MAC_CHECK_PROBE2(bpfdesc_check_receive, error, d, ifp); - MAC_IFNET_UNLOCK(ifp); + MAC_IFNET_UNLOCK(ifp, locked); return (error); } @@ -376,7 +383,7 @@ int mac_ifnet_check_transmit(struct ifnet *ifp, struct mbuf *m) { struct label *label; - int error; + int error, locked; M_ASSERTPKTHDR(m); @@ -385,11 +392,11 @@ mac_ifnet_check_transmit(struct ifnet *ifp, struct mbu label = mac_mbuf_to_label(m); - MAC_IFNET_LOCK(ifp); + MAC_IFNET_LOCK(ifp, locked); MAC_POLICY_CHECK_NOSLEEP(ifnet_check_transmit, ifp, ifp->if_label, m, label); MAC_CHECK_PROBE2(ifnet_check_transmit, error, ifp, m); - MAC_IFNET_UNLOCK(ifp); + MAC_IFNET_UNLOCK(ifp, locked); return (error); } @@ -401,7 +408,7 @@ mac_ifnet_ioctl_get(struct ucred *cred, struct ifreq * char *elements, *buffer; struct label *intlabel; struct mac mac; - int error; + int error, locked; if (!(mac_labeled & MPC_OBJECT_IFNET)) return (EINVAL); @@ -423,9 +430,9 @@ mac_ifnet_ioctl_get(struct ucred *cred, struct ifreq * buffer = malloc(mac.m_buflen, M_MACTEMP, M_WAITOK | M_ZERO); intlabel = mac_ifnet_label_alloc(); - MAC_IFNET_LOCK(ifp); + MAC_IFNET_LOCK(ifp, locked); mac_ifnet_copy_label(ifp->if_label, intlabel); - MAC_IFNET_UNLOCK(ifp); + MAC_IFNET_UNLOCK(ifp, locked); error = mac_ifnet_externalize_label(intlabel, elements, buffer, mac.m_buflen); mac_ifnet_label_free(intlabel); @@ -444,7 +451,7 @@ mac_ifnet_ioctl_set(struct ucred *cred, struct ifreq * struct label *intlabel; struct mac mac; char *buffer; - int error; + int error, locked; if (!(mac_labeled & MPC_OBJECT_IFNET)) return (EINVAL); @@ -483,18 +490,18 @@ mac_ifnet_ioctl_set(struct ucred *cred, struct ifreq * return (error); } - MAC_IFNET_LOCK(ifp); + MAC_IFNET_LOCK(ifp, locked); MAC_POLICY_CHECK_NOSLEEP(ifnet_check_relabel, cred, ifp, ifp->if_label, intlabel); if (error) { - MAC_IFNET_UNLOCK(ifp); + MAC_IFNET_UNLOCK(ifp, locked); mac_ifnet_label_free(intlabel); return (error); } MAC_POLICY_PERFORM_NOSLEEP(ifnet_relabel, cred, ifp, ifp->if_label, intlabel); - MAC_IFNET_UNLOCK(ifp); + MAC_IFNET_UNLOCK(ifp, locked); mac_ifnet_label_free(intlabel); return (0);