git: 10729d0ce11c - main - net: Fix handling of sockaddrs in the SIOC{ADD,DEL}MULTI handlers
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
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: