From nobody Tue Sep 30 03:07:15 2025 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4cbNJr3TLQz69HS2; Tue, 30 Sep 2025 03:07:24 +0000 (UTC) (envelope-from zlei@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [IPv6:2610:1c1:1:606c::24b:4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R13" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4cbNJr2s1Cz3gx2; Tue, 30 Sep 2025 03:07:24 +0000 (UTC) (envelope-from zlei@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1759201644; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=+AKObhsLT0HhU5UFt4PJ02XrLbJ4r9sNShUMB8o01SE=; b=EGc/iC5lVAQECo/pLO+MLJCAxIEdWtUmsYGK22pwXqAKcjvwVxbWc0wMAGtSQl05+UuNxa TF+YHL2i//4Zx4rAOWSuYZsyD85bxz/HkFiQ2X7KVZymN2LKky+MIipBLp6sbHgyHxepHH WxATWBt1Sbj1MJyH5392SZPLhMOlaNJprAIQzLY8fwq+qGwgdtAzjTWfiOqknvaX/OWPoc wSeakTtcX53fh5GjQR9uKiDvo4yM4KynYtDa1U2vmY/xt0eG9OHv6egGiJZvBpp5wiPLZt Xs84ki4d1cMC3uCcDKzVM/QiZH5AtQ5esIobJj0NmB29w2RauwyZyarIcrM+Fw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1759201644; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=+AKObhsLT0HhU5UFt4PJ02XrLbJ4r9sNShUMB8o01SE=; b=tMPsgJS6sJWmRKi61qr5zWg4teR0ibQVUT5YdjN25WajJVu5A2kCGJgz1v7K5KKk+6YVsT fZKCfzNRXx6MBNpJp2haa/F5V6eQK5FPCzd806Ijq1kD6u/wM0oJFwuHHtpb6Pd5dzpW1K 14P99F6VN3hqpOy2EURO7LjBYRPbn06MpOadKREeovyI4GWkeIrYsWJ56C1a6gZ7JrmOFg pJBm+o0wJ08VpBXRn95ZGkSezFSAJz34KR0oARI9kisdGLet5gWD17eoNfempP402VDs/Q oHPILbUvc+CLT7HR3L5ad/YR+XgrV+ROLPZ7U25i2YPUcGLrZ5Eawcsif7427A== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1759201644; a=rsa-sha256; cv=none; b=UhosC1VAyi1RMGm6r2A9Ys4vZDXX+oSBdjAYl/KPmZ3TMno2YjDQCnFbeFgDPjTP6AIbYy Xbk6APAwhhooMzSRdjoSh+syYXPFtxqhqUbNLAqasrdOJUYWYhumxG6oYTXIMhcvjbbZZh K/WrwCk3hL8b+BU/nxU0k1KxQlKVLjb/W1liXnmAN8dFfFCej5tqgwbaEolOZ/JbSnoDO4 caXeJ1wntdCd58Mgs9d4B+tVKWIbGgz5+523a1MbwgPXFfulKaQm2nM/rcrwEV0QnR3UCk gBuD6Tvem5e2dpk2VkMjbWt7dGpy8m2AaE8ExYDHDpPsofGzVtbrRjqIHTT7hA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none Received: from smtpclient.apple (ns1.oxydns.net [45.32.91.63]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) (Authenticated sender: zlei/mail) by smtp.freebsd.org (Postfix) with ESMTPSA id 4cbNJn6HCYzPZ4; Tue, 30 Sep 2025 03:07:21 +0000 (UTC) (envelope-from zlei@FreeBSD.org) From: Zhenlei Huang Message-Id: <13C41285-E7E1-4B49-B9A7-1B157B445CDD@FreeBSD.org> Content-Type: multipart/alternative; boundary="Apple-Mail=_8A8C8FF2-DB0E-4602-97BC-9632550436C9" List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@FreeBSD.org Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3696.120.41.1.10\)) Subject: Re: svn commit: r256519 - in head/sys: net netatalk netinet netinet6 netipx Date: Tue, 30 Sep 2025 11:07:15 +0800 In-Reply-To: Cc: Gleb Smirnoff , src-committers@freebsd.org, "" , "dev-commits-src-main@FreeBSD.org" , "Bjoern A. Zeeb" , Kristof Provost To: Mateusz Guzik References: <201310151031.r9FAVgRP008282@svn.freebsd.org> <35D99D40-5534-402A-8479-64C71604206B@FreeBSD.org> X-Mailer: Apple Mail (2.3696.120.41.1.10) --Apple-Mail=_8A8C8FF2-DB0E-4602-97BC-9632550436C9 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 > On Sep 30, 2025, at 10:59 AM, Mateusz Guzik wrote: >=20 > On Tue, Sep 30, 2025 at 4:38=E2=80=AFAM Zhenlei Huang = > wrote: >>=20 >>=20 >>=20 >>> On Sep 29, 2025, at 10:29 PM, Mateusz Guzik = wrote: >>>=20 >>> On Tue, Oct 15, 2013 at 12:31=E2=80=AFPM Gleb Smirnoff = wrote: >>>>=20 >>>> Author: glebius >>>> Date: Tue Oct 15 10:31:42 2013 >>>> New Revision: 256519 >>>> URL: http://svnweb.freebsd.org/changeset/base/256519 >>>>=20 >>>> Log: >>>> Remove ifa_init() and provide ifa_alloc() that will allocate and = setup >>>> struct ifaddr internally. >>>>=20 >>>> Sponsored by: Netflix >>>> Sponsored by: Nginx, Inc. >>>>=20 >>>> 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 >>>>=20 >>>> 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)); >>>> + >>>=20 >>> We have crashes stemming from this: >>>=20 >>> panic: ifa_alloc: invalid size 16 >>>=20 >>> 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 >>>=20 >>> in6_ifadd has: >>> struct in6_addr taddr; >>> ifa =3D ifa_alloc(sizeof(taddr), M_WAITOK); >>>=20 >>> should the assert be simply removed? >>=20 >> Hi Mateusz, >>=20 >> I believe you just found a bug. >>=20 >> Try the following patch please, >>=20 >> --- a/sys/netinet6/nd6_rtr.c >> +++ b/sys/netinet6/nd6_rtr.c >> @@ -1243,8 +1243,7 @@ in6_ifadd(struct nd_prefixctl *pr, int mcast) >>=20 >> /* No suitable LL address, get the ifid directly */ >> if (ifid_addr =3D=3D NULL) { >> - struct in6_addr taddr; >> - ifa =3D ifa_alloc(sizeof(taddr), M_WAITOK); >> + ifa =3D ifa_alloc(sizeof(struct in6_ifaddr), = M_WAITOK); >> if (ifa) { >> ib =3D (struct in6_ifaddr *)ifa; >> ifid_addr =3D &ib->ia_addr.sin6_addr; >>=20 >=20 > Thanks for the patch. I don't have means to readily test it. >=20 > This panic was getting in the way of looking at another panic so I did > not pay much attention. >=20 > But now that you point this out, I don't think the patch is = sufficient. >=20 > in6_get_ifid starts with NET_EPOCH_ASSERT. >=20 > At the same time malloc(..., M_WAITOK) is illegal to call from an = epoch section. So M_NOWAIT should be used, instead of M_WAITOK . >=20 > I don't know if in6_ifadd is called within net epoch, either way the > above two are clearly contradictory. >=20 > Is there are a reason to malloc this in the first place? I have not look into the code throughly. That was introduced via commit = https://cgit.freebsd.org/src/commit/?id=3D9e792f7ef7298080c058fbc2d36a4e60= e596dae9 . See https://reviews.freebsd.org/D51778 for more context. Best regards, Zhenlei --Apple-Mail=_8A8C8FF2-DB0E-4602-97BC-9632550436C9 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8

On Sep 30, 2025, at 10:59 AM, Mateusz Guzik <mjguzik@gmail.com> = wrote:

On Tue, Sep 30, 2025 at 4:38=E2=80=AFAM Zhenlei Huang = <zlei@freebsd.org> = wrote:



On Sep 29, 2025, at 10:29 PM, Mateusz Guzik <mjguzik@gmail.com> = wrote:

On Tue, Oct 15, 2013 at 12:31=E2=80=AF= 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
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=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
          &nb= sp;           socks= ize =3D sizeof(*sdl);
          &nb= sp;   socksize =3D roundup2(socksize, sizeof(long));
          &nb= sp;   ifasize =3D sizeof(*ifa) + 2 * socksize;
- =             &n= bsp; ifa =3D malloc(ifasize, M_IFADDR, M_WAITOK | M_ZERO);
- =             &n= bsp; ifa_init(ifa);
+ =             &n= bsp; ifa =3D ifa_alloc(ifasize, M_WAITOK);
          &nb= sp;   sdl =3D (struct sockaddr_dl *)(ifa + 1);
          &nb= sp;   sdl->sdl_len =3D socksize;
          &nb= sp;   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:
          &nb= sp;           struc= t in6_addr taddr;
          &nb= sp;           ifa = =3D 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)

          &nb= sp;    /* No suitable LL address, get the ifid = directly */
          &nb= sp;    if (ifid_addr =3D=3D NULL) {
- =             &n= bsp;         struct = in6_addr taddr;
- =             &n= bsp;         ifa =3D = ifa_alloc(sizeof(taddr), M_WAITOK);
+ =             &n= bsp;         ifa =3D = ifa_alloc(sizeof(struct in6_ifaddr), M_WAITOK);
          &nb= sp;            = ;if (ifa) {
          &nb= sp;            = ;        ib =3D (struct = in6_ifaddr *)ifa;
          &nb= sp;            = ;        ifid_addr =3D = &ib->ia_addr.sin6_addr;


Thanks for the patch. I don't have means to readily test = it.

This panic = was getting in the way of looking at another panic so I did
not pay much = attention.

But now that = you point this out, I don't think the patch is sufficient.

in6_get_ifid = starts with NET_EPOCH_ASSERT.

At the same time malloc(..., M_WAITOK) is illegal to call = from an epoch section.

So M_NOWAIT should be used, instead of M_WAITOK = .


I don't know = if in6_ifadd is called within net epoch, either way the
above two are = clearly contradictory.

Is there are a reason to malloc this in the first = place?

I have not = look into the code throughly. That was introduced via commit https://cgit.freebsd.org/src/commit/?id=3D9e792f7ef7298080c058f= bc2d36a4e60e596dae9 .

See https://reviews.freebsd.org/D51778 for more = context.

Best regards,
Zhenlei

= --Apple-Mail=_8A8C8FF2-DB0E-4602-97BC-9632550436C9--