git: 431856c868de - main - netlink: Bypass refcounting when setting promiscuity

The branch main has been updated by obiwac:

URL: https://cgit.FreeBSD.org/src/commit/?id=431856c868de12d7af127cdf7e9aeb1b99d0ac99

commit 431856c868de12d7af127cdf7e9aeb1b99d0ac99
Author:     Aymeric Wibo <obiwac@FreeBSD.org>
AuthorDate: 2025-08-25 22:40:16 +0000
Commit:     Aymeric Wibo <obiwac@FreeBSD.org>
CommitDate: 2025-08-25 22:40:17 +0000

    netlink: Bypass refcounting when setting promiscuity
    
    When asking for IFF_PROMISC when modifying interfaces with netlink, set
    permanent flag instead (IFF_PPROMISC) as netlink interface modification
    has no way of doing promiscuity reference counting through ifpromisc().
    We can't do reference counting because every netlink interface
    modification necessarily either sets or unsets IFF_PROMISC in ifi_flags,
    and ifi_change is usually set to 0xFFFFFFFF.
    
    This logic was the same between this and SIOCSIFFLAGS, so factor out
    if_setppromisc() function.
    
    Reviewed by:    melifaro, saheed, kp, mckusick (mentor)
    Approved by:    melifaro, saheed, mckusick (mentor)
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D52056
---
 sys/net/if.c                      | 37 +++++++++++++++++++++++++++----------
 sys/net/if_var.h                  |  1 +
 sys/netlink/route/iface_drivers.c | 17 ++++++++++-------
 3 files changed, 38 insertions(+), 17 deletions(-)

diff --git a/sys/net/if.c b/sys/net/if.c
index 79c883fd4a0a..202be4794f6e 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -2589,16 +2589,7 @@ ifhwioctl(u_long cmd, struct ifnet *ifp, caddr_t data, struct thread *td)
 		 * flip.  They require special handling because in-kernel
 		 * consumers may indepdently toggle them.
 		 */
-		if ((ifp->if_flags ^ new_flags) & IFF_PPROMISC) {
-			if (new_flags & IFF_PPROMISC)
-				ifp->if_flags |= IFF_PROMISC;
-			else if (ifp->if_pcount == 0)
-				ifp->if_flags &= ~IFF_PROMISC;
-			if (log_promisc_mode_change)
-                                if_printf(ifp, "permanently promiscuous mode %s\n",
-                                    ((new_flags & IFF_PPROMISC) ?
-                                     "enabled" : "disabled"));
-		}
+		if_setppromisc(ifp, new_flags & IFF_PPROMISC);
 		if ((ifp->if_flags ^ new_flags) & IFF_PALLMULTI) {
 			if (new_flags & IFF_PALLMULTI)
 				ifp->if_flags |= IFF_ALLMULTI;
@@ -4456,6 +4447,32 @@ if_getmtu_family(const if_t ifp, int family)
 	return (ifp->if_mtu);
 }
 
+void
+if_setppromisc(if_t ifp, bool ppromisc)
+{
+	int new_flags;
+
+	if (ppromisc)
+		new_flags = ifp->if_flags | IFF_PPROMISC;
+	else
+		new_flags = ifp->if_flags & ~IFF_PPROMISC;
+	if ((ifp->if_flags ^ new_flags) & IFF_PPROMISC) {
+		if (new_flags & IFF_PPROMISC)
+			new_flags |= IFF_PROMISC;
+		/*
+		 * Only unset IFF_PROMISC if there are no more consumers of
+		 * promiscuity, i.e. the ifp->if_pcount refcount is 0.
+		 */
+		else if (ifp->if_pcount == 0)
+			new_flags &= ~IFF_PROMISC;
+		if (log_promisc_mode_change)
+                        if_printf(ifp, "permanently promiscuous mode %s\n",
+                            ((new_flags & IFF_PPROMISC) ?
+                             "enabled" : "disabled"));
+	}
+	ifp->if_flags = new_flags;
+}
+
 /*
  * Methods for drivers to access interface unicast and multicast
  * link level addresses.  Driver shall not know 'struct ifaddr' neither
diff --git a/sys/net/if_var.h b/sys/net/if_var.h
index 08435e7bd5f6..f2df612b19c1 100644
--- a/sys/net/if_var.h
+++ b/sys/net/if_var.h
@@ -622,6 +622,7 @@ int if_setmtu(if_t ifp, int mtu);
 int if_getmtu(const if_t ifp);
 int if_getmtu_family(const if_t ifp, int family);
 void if_notifymtu(if_t ifp);
+void if_setppromisc(const if_t ifp, bool ppromisc);
 int if_setflagbits(if_t ifp, int set, int clear);
 int if_setflags(if_t ifp, int flags);
 int if_getflags(const if_t ifp);
diff --git a/sys/netlink/route/iface_drivers.c b/sys/netlink/route/iface_drivers.c
index 2a75e6abb2d0..4f1540740ead 100644
--- a/sys/netlink/route/iface_drivers.c
+++ b/sys/netlink/route/iface_drivers.c
@@ -105,13 +105,16 @@ _nl_modify_ifp_generic(struct ifnet *ifp, struct nl_parsed_link *lattrs,
 		}
 	}
 
-	if (lattrs->ifi_change & IFF_PROMISC) {
-		error = ifpromisc(ifp, lattrs->ifi_flags & IFF_PROMISC);
-		if (error != 0) {
-			nlmsg_report_err_msg(npt, "unable to set promisc");
-			return (error);
-		}
-	}
+	if ((lattrs->ifi_change & IFF_PROMISC) != 0 ||
+	    lattrs->ifi_change == 0)
+		/*
+		 * When asking for IFF_PROMISC, set permanent flag instead
+		 * (IFF_PPROMISC) as we have no way of doing promiscuity
+		 * reference counting through ifpromisc().  Every call to this
+		 * function either sets or unsets IFF_PROMISC, and ifi_change
+		 * is usually set to 0xFFFFFFFF.
+		 */
+		if_setppromisc(ifp, (lattrs->ifi_flags & IFF_PROMISC) != 0);
 
 	if (lattrs->ifla_address != NULL) {
 		if (nlp_has_priv(npt->nlp, PRIV_NET_SETIFMAC)) {