Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 29 Sep 2025 16:29:10 +0200
From:      Mateusz Guzik <mjguzik@gmail.com>
To:        Gleb Smirnoff <glebius@freebsd.org>
Cc:        src-committers@freebsd.org,  "<dev-commits-src-all@freebsd.org>" <dev-commits-src-all@freebsd.org>,  "dev-commits-src-main@FreeBSD.org" <dev-commits-src-main@freebsd.org>
Subject:   Re: svn commit: r256519 - in head/sys: net netatalk netinet netinet6 netipx
Message-ID:  <CAGudoHGN=6-dFQ791_=kLBnYfyz9zm75-vJKh=e9gibaXun5NQ@mail.gmail.com>
In-Reply-To: <201310151031.r9FAVgRP008282@svn.freebsd.org>
References:  <201310151031.r9FAVgRP008282@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Oct 15, 2013 at 12:31=E2=80=AFPM 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 setu=
p
>   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
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> --- 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 =3D sizeof(*sdl);
>                 socksize =3D roundup2(socksize, sizeof(long));
>                 ifasize =3D sizeof(*ifa) + 2 * socksize;
> -               ifa =3D malloc(ifasize, M_IFADDR, M_WAITOK | M_ZERO);
> -               ifa_init(ifa);
> +               ifa =3D ifa_alloc(ifasize, M_WAITOK);
>                 sdl =3D (struct sockaddr_dl *)(ifa + 1);
>                 sdl->sdl_len =3D socksize;
>                 sdl->sdl_family =3D 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 >=3D 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 =3D ifa_alloc(sizeof(taddr), M_WAITOK);

should the assert be simply removed?

> +       ifa =3D malloc(size, M_IFADDR, M_ZERO | flags);
> +       if (ifa =3D=3D NULL)
> +               return (NULL);
>
>         mtx_init(&ifa->ifa_mtx, "ifaddr", NULL, MTX_DEF);
>         refcount_init(&ifa->ifa_refcnt, 1);
>         ifa->if_data.ifi_datalen =3D sizeof(ifa->if_data);
> +
> +       return (ifa);
>  }
>
>  void
>
> Modified: head/sys/net/if_var.h
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> --- 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
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> --- 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 =3D=3D NULL) {
> -                       aa =3D malloc(sizeof(struct at_ifaddr), M_IFADDR,
> -                           M_NOWAIT | M_ZERO);
> -                       if (aa =3D=3D NULL) {
> -                               error =3D ENOBUFS;
> -                               goto out;
> -                       }
> -                       callout_init(&aa->aa_callout, CALLOUT_MPSAFE);
> +                       ifa =3D ifa_alloc(sizeof(struct at_ifaddr), M_WAI=
TOK);
> +                       aa =3D (struct at_ifaddr *)ifa;
>
> -                       ifa =3D (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
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> --- 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 =3D=3D NULL) {
> -                       ia =3D (struct in_ifaddr *)
> -                               malloc(sizeof *ia, M_IFADDR, M_NOWAIT |
> -                                   M_ZERO);
> -                       if (ia =3D=3D NULL) {
> -                               error =3D ENOBUFS;
> -                               goto out;
> -                       }
> -
> -                       ifa =3D &ia->ia_ifa;
> -                       ifa_init(ifa);
> +                       ifa =3D ifa_alloc(sizeof(struct in_ifaddr), M_WAI=
TOK);
> +                       ia =3D (struct in_ifaddr *)ifa;
>                         ifa->ifa_addr =3D (struct sockaddr *)&ia->ia_addr=
;
>                         ifa->ifa_dstaddr =3D (struct sockaddr *)&ia->ia_d=
staddr;
>                         ifa->ifa_netmask =3D (struct sockaddr *)&ia->ia_s=
ockmask;
>
> Modified: head/sys/netinet6/in6.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> --- 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 s=
hould
>                  * call malloc with M_NOWAIT.
>                  */
> -               ia =3D (struct in6_ifaddr *) malloc(sizeof(*ia), M_IFADDR=
,
> -                   M_NOWAIT);
> +               ia =3D (struct in6_ifaddr *)ifa_alloc(sizeof(*ia), M_NOWA=
IT);
>                 if (ia =3D=3D 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 =3D (struct sockaddr *)&ia->ia_addr;
>
> Modified: head/sys/netipx/ipx.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> --- 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 =3D priv_check(td, PRIV_NET_SETLLADDR)) =
!=3D 0)
>                         goto out;
>                 if (ia =3D=3D NULL) {
> -                       ia =3D malloc(sizeof(*ia), M_IFADDR, M_NOWAIT | M=
_ZERO);
> -                       if (ia =3D=3D NULL) {
> -                               error =3D ENOBUFS;
> -                               goto out;
> -                       }
> -                       ifa =3D (struct ifaddr *)ia;
> -                       ifa_init(ifa);
> +                       ifa =3D ifa_alloc(sizeof(struct ipx_ifaddr), M_WA=
ITOK);
> +                       ia =3D (struct ipx_ifaddr *)ifa;
>                         ia->ia_ifp =3D ifp;
>                         ifa->ifa_addr =3D (struct sockaddr *)&ia->ia_addr=
;
>                         ifa->ifa_netmask =3D (struct sockaddr *)&ipx_netm=
ask;



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAGudoHGN=6-dFQ791_=kLBnYfyz9zm75-vJKh=e9gibaXun5NQ>