git: 508c057281e9 - stable/14 - net: Fix handling of sockaddrs in the SIOC{ADD,DEL}MULTI handlers

From: Mark Johnston <markj_at_FreeBSD.org>
Date: Mon, 26 Oct 2026 19:46:51 UTC
The branch stable/14 has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=508c057281e93a368e5bf5e41beca5dc2d74f772

commit 508c057281e93a368e5bf5e41beca5dc2d74f772
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2026-09-29 18:50:02 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2026-10-26 14:12:15 +0000

    net: Fix handling of sockaddrs in the SIOC{ADD,DEL}MULTI handlers
    
    The SIOCADDMULTI and SIOCDELMULTI handlers add or delete a link-layer
    multicast address from an interface's multicast filter list.  The
    link-layer address is passed using the ifr_addr field of the request
    structure.
    
    struct ifreq's ifr_addr field is a struct sockaddr, which is a fair bit
    smaller than struct sockaddr_dl (though big enough to hold an ethernet
    address).  Existing callers set the sockaddr length to
    sizeof(struct sockaddr_dl), which is too large, and causes OOB accesses
    when if_findmulti() is used to compare the address with others, or when
    if_addmulti() makes a copy.
    
    Fix this without breaking compatibility: copy the user-supplied address
    into a sockaddr_dl on the stack, and use the latter for the respective
    operation.
    
    Also validate the sockaddr_dl internal length fields, suggested by zlei.
    
    Reported by:    Yuxiang Yang, Yizhou Zhao, Ao Wang, Xuewei Feng, Qi Li,
                    and Ke Xu from Tsinghua University using GLM-5.2 from Z.ai
    Reviewed by:    zlei, ae, glebius
    MFC after:      1 week
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D59919
    
    (cherry picked from commit 10729d0ce11c9ac5077d158bb372f9c39053f168)
---
 sys/net/if.c | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/sys/net/if.c b/sys/net/if.c
index 7856f53a3cea..e0cdf7c04674 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -2776,6 +2776,9 @@ ifhwioctl(u_long cmd, struct ifnet *ifp, caddr_t data, struct thread *td)
 
 	case SIOCADDMULTI:
 	case SIOCDELMULTI:
+	{
+		struct sockaddr_dl sa_dl;
+
 		if (cmd == SIOCADDMULTI)
 			error = priv_check(td, PRIV_NET_ADDMULTI);
 		else
@@ -2787,9 +2790,25 @@ ifhwioctl(u_long cmd, struct ifnet *ifp, caddr_t data, struct thread *td)
 		if ((ifp->if_flags & IFF_MULTICAST) == 0)
 			return (EOPNOTSUPP);
 
-		/* Don't let users screw up protocols' entries. */
+		/*
+		 * The sockaddr embedded in the ifreq is not large enough to
+		 * hold a full sockaddr_dl, but existing callers of these ioctls
+		 * will set sa_len = sizeof(struct sockaddr_dl) anyway.  Bounce
+		 * the sockaddr into a sockaddr_dl on the stack to avoid
+		 * potential out-of-bounds accesses.
+		 */
 		if (ifr->ifr_addr.sa_family != AF_LINK)
 			return (EINVAL);
+		memset(&sa_dl, 0, sizeof(sa_dl));
+		memcpy(&sa_dl, &ifr->ifr_addr,
+		    MIN(ifr->ifr_addr.sa_len, sizeof(ifr->ifr_addr)));
+		sa_dl.sdl_family = AF_LINK;
+		sa_dl.sdl_len = MIN(ifr->ifr_addr.sa_len, sizeof(sa_dl));
+		if (sa_dl.sdl_nlen + sa_dl.sdl_alen + sa_dl.sdl_slen >
+		    sizeof(ifr->ifr_addr) -
+		    offsetof(struct sockaddr_dl, sdl_data) ||
+		    sa_dl.sdl_nlen > IFNAMSIZ)
+			return (EINVAL);
 
 		if (cmd == SIOCADDMULTI) {
 			struct epoch_tracker et;
@@ -2803,18 +2822,20 @@ ifhwioctl(u_long cmd, struct ifnet *ifp, caddr_t data, struct thread *td)
 			 * already exists.
 			 */
 			NET_EPOCH_ENTER(et);
-			ifma = if_findmulti(ifp, &ifr->ifr_addr);
+			ifma = if_findmulti(ifp, (struct sockaddr *)&sa_dl);
 			NET_EPOCH_EXIT(et);
 			if (ifma != NULL)
 				error = EADDRINUSE;
 			else
-				error = if_addmulti(ifp, &ifr->ifr_addr, &ifma);
+				error = if_addmulti(ifp,
+				    (struct sockaddr *)&sa_dl, &ifma);
 		} else {
-			error = if_delmulti(ifp, &ifr->ifr_addr);
+			error = if_delmulti(ifp, (struct sockaddr *)&sa_dl);
 		}
 		if (error == 0)
 			getmicrotime(&ifp->if_lastchange);
 		break;
+	}
 
 	case SIOCSIFPHYADDR:
 	case SIOCDIFPHYADDR: