Re: svn commit: r256519 - in head/sys: net netatalk netinet netinet6 netipx
Date: Tue, 30 Sep 2025 02:38:10 UTC
> On Sep 29, 2025, at 10:29 PM, Mateusz Guzik <mjguzik@gmail.com> wrote:
>
> On Tue, Oct 15, 2013 at 12:31 PM Gleb Smirnoff <glebius@freebsd.org> wrote:
>>
>> Author: glebius
>> Date: Tue Oct 15 10:31:42 2013
>> New Revision: 256519
>> URL: http://svnweb.freebsd.org/changeset/base/256519
>>
>> Log:
>> Remove ifa_init() and provide ifa_alloc() that will allocate and setup
>> struct ifaddr internally.
>>
>> Sponsored by: Netflix
>> Sponsored by: Nginx, Inc.
>>
>> Modified:
>> head/sys/net/if.c
>> head/sys/net/if_var.h
>> head/sys/netatalk/at_control.c
>> head/sys/netinet/in.c
>> head/sys/netinet6/in6.c
>> head/sys/netipx/ipx.c
>>
>> Modified: head/sys/net/if.c
>> ==============================================================================
>> --- head/sys/net/if.c Tue Oct 15 10:19:24 2013 (r256518)
>> +++ head/sys/net/if.c Tue Oct 15 10:31:42 2013 (r256519)
>> @@ -633,8 +633,7 @@ if_attach_internal(struct ifnet *ifp, in
>> socksize = sizeof(*sdl);
>> socksize = roundup2(socksize, sizeof(long));
>> ifasize = sizeof(*ifa) + 2 * socksize;
>> - ifa = malloc(ifasize, M_IFADDR, M_WAITOK | M_ZERO);
>> - ifa_init(ifa);
>> + ifa = ifa_alloc(ifasize, M_WAITOK);
>> sdl = (struct sockaddr_dl *)(ifa + 1);
>> sdl->sdl_len = socksize;
>> sdl->sdl_family = AF_LINK;
>> @@ -1417,13 +1416,23 @@ if_maddr_runlock(struct ifnet *ifp)
>> /*
>> * Initialization, destruction and refcounting functions for ifaddrs.
>> */
>> -void
>> -ifa_init(struct ifaddr *ifa)
>> +struct ifaddr *
>> +ifa_alloc(size_t size, int flags)
>> {
>> + struct ifaddr *ifa;
>> +
>> + KASSERT(size >= sizeof(struct ifaddr),
>> + ("%s: invalid size %zu", __func__, size));
>> +
>
> We have crashes stemming from this:
>
> panic: ifa_alloc: invalid size 16
>
> panic() at panic+0x43/frame 0xfffffe009e777760
> ifa_alloc() at ifa_alloc+0xd6/frame 0xfffffe009e777780
> in6_ifadd() at in6_ifadd+0xd8/frame 0xfffffe009e7778a0
> nd6_ra_input() at nd6_ra_input+0x1023/frame 0xfffffe009e777a80
> icmp6_input() at icmp6_input+0x5b6/frame 0xfffffe009e777c00
> ip6_input() at ip6_input+0xc94/frame 0xfffffe009e777ce0
> sppp_input() at sppp_input+0x502/frame 0xfffffe009e777d70
> pppoe_data_input() at pppoe_data_input+0x1e7/frame 0xfffffe009e777de0
> swi_net() at swi_net+0x19b/frame 0xfffffe009e777e60
> ithread_loop() at ithread_loop+0x266/frame 0xfffffe009e777ef0
> fork_exit() at fork_exit+0x82/frame 0xfffffe009e777f30
> fork_trampoline() at fork_trampoline+0xe/frame 0xfffffe009e777f30
>
> in6_ifadd has:
> struct in6_addr taddr;
> ifa = ifa_alloc(sizeof(taddr), M_WAITOK);
>
> should the assert be simply removed?
Hi Mateusz,
I believe you just found a bug.
Try the following patch please,
--- a/sys/netinet6/nd6_rtr.c
+++ b/sys/netinet6/nd6_rtr.c
@@ -1243,8 +1243,7 @@ in6_ifadd(struct nd_prefixctl *pr, int mcast)
/* No suitable LL address, get the ifid directly */
if (ifid_addr == NULL) {
- struct in6_addr taddr;
- ifa = ifa_alloc(sizeof(taddr), M_WAITOK);
+ ifa = ifa_alloc(sizeof(struct in6_ifaddr), M_WAITOK);
if (ifa) {
ib = (struct in6_ifaddr *)ifa;
ifid_addr = &ib->ia_addr.sin6_addr;
Best regards,
Zhenlei
>
>> + ifa = malloc(size, M_IFADDR, M_ZERO | flags);
>> + if (ifa == NULL)
>> + return (NULL);
>>
>> mtx_init(&ifa->ifa_mtx, "ifaddr", NULL, MTX_DEF);
>> refcount_init(&ifa->ifa_refcnt, 1);
>> ifa->if_data.ifi_datalen = sizeof(ifa->if_data);
>> +
>> + return (ifa);
>> }
>>
>> void
>>
>> Modified: head/sys/net/if_var.h
>> ==============================================================================
>> --- head/sys/net/if_var.h Tue Oct 15 10:19:24 2013 (r256518)
>> +++ head/sys/net/if_var.h Tue Oct 15 10:31:42 2013 (r256519)
>> @@ -819,8 +819,8 @@ struct ifaddr {
>> #define IFA_LOCK(ifa) mtx_lock(&(ifa)->ifa_mtx)
>> #define IFA_UNLOCK(ifa) mtx_unlock(&(ifa)->ifa_mtx)
>>
>> +struct ifaddr * ifa_alloc(size_t size, int flags);
>> void ifa_free(struct ifaddr *ifa);
>> -void ifa_init(struct ifaddr *ifa);
>> void ifa_ref(struct ifaddr *ifa);
>> #endif
>>
>>
>> Modified: head/sys/netatalk/at_control.c
>> ==============================================================================
>> --- head/sys/netatalk/at_control.c Tue Oct 15 10:19:24 2013 (r256518)
>> +++ head/sys/netatalk/at_control.c Tue Oct 15 10:31:42 2013 (r256519)
>> @@ -199,16 +199,10 @@ at_control(struct socket *so, u_long cmd
>> * allocate a fresh one.
>> */
>> if (aa == NULL) {
>> - aa = malloc(sizeof(struct at_ifaddr), M_IFADDR,
>> - M_NOWAIT | M_ZERO);
>> - if (aa == NULL) {
>> - error = ENOBUFS;
>> - goto out;
>> - }
>> - callout_init(&aa->aa_callout, CALLOUT_MPSAFE);
>> + ifa = ifa_alloc(sizeof(struct at_ifaddr), M_WAITOK);
>> + aa = (struct at_ifaddr *)ifa;
>>
>> - ifa = (struct ifaddr *)aa;
>> - ifa_init(ifa);
>> + callout_init(&aa->aa_callout, CALLOUT_MPSAFE);
>>
>> /*
>> * As the at_ifaddr contains the actual sockaddrs,
>>
>> Modified: head/sys/netinet/in.c
>> ==============================================================================
>> --- head/sys/netinet/in.c Tue Oct 15 10:19:24 2013 (r256518)
>> +++ head/sys/netinet/in.c Tue Oct 15 10:31:42 2013 (r256519)
>> @@ -404,16 +404,8 @@ in_control(struct socket *so, u_long cmd
>> goto out;
>> }
>> if (ia == NULL) {
>> - ia = (struct in_ifaddr *)
>> - malloc(sizeof *ia, M_IFADDR, M_NOWAIT |
>> - M_ZERO);
>> - if (ia == NULL) {
>> - error = ENOBUFS;
>> - goto out;
>> - }
>> -
>> - ifa = &ia->ia_ifa;
>> - ifa_init(ifa);
>> + ifa = ifa_alloc(sizeof(struct in_ifaddr), M_WAITOK);
>> + ia = (struct in_ifaddr *)ifa;
>> ifa->ifa_addr = (struct sockaddr *)&ia->ia_addr;
>> ifa->ifa_dstaddr = (struct sockaddr *)&ia->ia_dstaddr;
>> ifa->ifa_netmask = (struct sockaddr *)&ia->ia_sockmask;
>>
>> Modified: head/sys/netinet6/in6.c
>> ==============================================================================
>> --- head/sys/netinet6/in6.c Tue Oct 15 10:19:24 2013 (r256518)
>> +++ head/sys/netinet6/in6.c Tue Oct 15 10:31:42 2013 (r256519)
>> @@ -1141,12 +1141,9 @@ in6_update_ifa(struct ifnet *ifp, struct
>> * RA, it is called under an interrupt context. So, we should
>> * call malloc with M_NOWAIT.
>> */
>> - ia = (struct in6_ifaddr *) malloc(sizeof(*ia), M_IFADDR,
>> - M_NOWAIT);
>> + ia = (struct in6_ifaddr *)ifa_alloc(sizeof(*ia), M_NOWAIT);
>> if (ia == NULL)
>> return (ENOBUFS);
>> - bzero((caddr_t)ia, sizeof(*ia));
>> - ifa_init(&ia->ia_ifa);
>> LIST_INIT(&ia->ia6_memberships);
>> /* Initialize the address and masks, and put time stamp */
>> ia->ia_ifa.ifa_addr = (struct sockaddr *)&ia->ia_addr;
>>
>> Modified: head/sys/netipx/ipx.c
>> ==============================================================================
>> --- head/sys/netipx/ipx.c Tue Oct 15 10:19:24 2013 (r256518)
>> +++ head/sys/netipx/ipx.c Tue Oct 15 10:31:42 2013 (r256519)
>> @@ -190,13 +190,8 @@ ipx_control(struct socket *so, u_long cm
>> if (td && (error = priv_check(td, PRIV_NET_SETLLADDR)) != 0)
>> goto out;
>> if (ia == NULL) {
>> - ia = malloc(sizeof(*ia), M_IFADDR, M_NOWAIT | M_ZERO);
>> - if (ia == NULL) {
>> - error = ENOBUFS;
>> - goto out;
>> - }
>> - ifa = (struct ifaddr *)ia;
>> - ifa_init(ifa);
>> + ifa = ifa_alloc(sizeof(struct ipx_ifaddr), M_WAITOK);
>> + ia = (struct ipx_ifaddr *)ifa;
>> ia->ia_ifp = ifp;
>> ifa->ifa_addr = (struct sockaddr *)&ia->ia_addr;
>> ifa->ifa_netmask = (struct sockaddr *)&ipx_netmask;