git: 10729d0ce11c - main - net: Fix handling of sockaddrs in the SIOC{ADD,DEL}MULTI handlers

From: Mark Johnston <markj_at_FreeBSD.org>
Date: Tue, 29 Sep 2026 18:55:36 UTC
The branch main has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=10729d0ce11c9ac5077d158bb372f9c39053f168

commit 10729d0ce11c9ac5077d158bb372f9c39053f168
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2026-09-29 18:50:02 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2026-09-29 18:50:02 +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
---
 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 084d0ffb8a65..d8184a528f08 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -2853,6 +2853,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
@@ -2864,9 +2867,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;
@@ -2880,18 +2899,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: