Date: Fri, 3 May 2019 20:38:43 +0000 (UTC) From: Robert Watson <rwatson@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r347058 - head/sys/security/mac Message-ID: <201905032038.x43KchLG065940@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
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);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201905032038.x43KchLG065940>