From nobody Sun Oct 5 09:53:16 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 4cfd5667Jzz69Py5; Sun, 05 Oct 2025 09:53:30 +0000 (UTC) (envelope-from kp@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 4cfd565QNjz3Qrl; Sun, 05 Oct 2025 09:53:30 +0000 (UTC) (envelope-from kp@freebsd.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1759658010; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Mj5Oehtj65OeZgACfM1wTxgl9j3FuJZZR5RYPqMD1Fo=; b=lYyVM/qsGykprNAkuX2Mx9nui2Mv+GIbBu8qm7+TeQ0wODxcyyPcElWCi8LADWsFXhlYkN galP8Iz0WC883/7+TDs8E4ErahrG3ylmovOPtAXXM1pE3ZvCmF8NLmB2it814G/DVXz0m3 hpuKnHDwSRZvCzbiDdGQ9oW0DflMCl4UUULu8B1D+GtaHoaf8GWal/ycE7O3+QlwrHPkGo as61UXw4vOyyHzKLyw6Nw2+Osx4QEjbRyZfAZyaOxEf1/d3E2qIKiBf/VU5yVFX0aMKBkV SwCO+JyCweHVv/b0fMvZeIALDTIcapaeYT9pkVL8iB3uMtC9rQML8z2B+qSwhw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1759658010; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Mj5Oehtj65OeZgACfM1wTxgl9j3FuJZZR5RYPqMD1Fo=; b=HO6S0lqOtZagbvCATsJ/s7P9+ORFqMwE9FkTQ8YcpLlDtHwF3PrdXXCTSmfkHfZUJGqidj QHo5KpXg+u8rVr3YZsygUQMWMTQlNVkkyCKNSpeJitVKCKRgsLewHI1E99w70lSuYXSNBU SggDwKo4QsBB0Mqzpp0Bgcfvd813mxiWzH5vbFBsg2GffZ9Mtw5otCF3u0Mjxl2nREDDxO MM4bfHH4+csZ5bJBv+Zk8/yHEwVq/v7HqOzasqVJNzcHYjTk3Z/onHrcO8hc15j8jtQyll /zH4CtDQzCHXDj9wKZ5RDKgTvUDCvzKcco5btR8XCdVXyjITCAhAaTiQFFoIMg== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1759658010; a=rsa-sha256; cv=none; b=uz3RXd/e529iO4LcJe8TDsW1pCvxt1olcveAjyjS/B8wMRXJ9CLPR+qYC3n+YtVWvm4iN+ yVeytde41yuHkN9kRiyOu6dxf8nqoZvZ6DdbF+hMccXeLGxlZg9VdaQ0j34belf3QHkTlx jK/JUouZEnTQcLXYsxEBjEwj20q2A15invQ26l6k3UBuzgr3B02QFvkvMaP71qI00AQsHE w0dsiSASHZdf/aXVSwXR7Q7BCFfyjLFBrt0vkD3rqpeC0i3z5F1eFihgFzwqz2Jb5uct5J PG1tSEZquOXqD5BKvjwolThjjy1B5qPr1qDM1EyWJvp9MbpN8W/XvQrC3rpJxA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none Received: from venus.codepro.be (venus.codepro.be [5.9.86.228]) (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 (2048 bits) client-digest SHA256) (Client CN "mx1.codepro.be", Issuer "R11" (verified OK)) (Authenticated sender: kp) by smtp.freebsd.org (Postfix) with ESMTPSA id 4cfd563WXJzWg; Sun, 05 Oct 2025 09:53:30 +0000 (UTC) (envelope-from kp@freebsd.org) Received: by venus.codepro.be (Postfix, authenticated sender kp) id 9022518B21; Sun, 05 Oct 2025 11:53:27 +0200 (CEST) Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable From: Kristof Provost 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 (1.0) Subject: Re: svn commit: r256519 - in head/sys: net netatalk netinet netinet6 netipx Date: Sun, 5 Oct 2025 11:53:16 +0200 Message-Id: References: Cc: Zhenlei Huang , Mateusz Guzik , Gleb Smirnoff , src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org In-Reply-To: To: "Bjoern A. Zeeb" X-Mailer: iPhone Mail (23A355) > On 4 Oct 2025, at 18:13, Bjoern A. Zeeb wrote: >=20 > =EF=BB=BFOn Tue, 30 Sep 2025, Zhenlei Huang wrote: >=20 >>>> 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 set= up >>>>>> 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 s= ection. >>=20 >> So M_NOWAIT should be used, instead of M_WAITOK . >>=20 >>>=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? >>=20 >> I have not look into the code throughly. That was introduced via commit h= ttps://cgit.freebsd.org/src/commit/?id=3D9e792f7ef7298080c058fbc2d36a4e60e59= 6dae9 . >>=20 >> See https://reviews.freebsd.org/D51778 for more context. >=20 > Wait, so first the bug came in on a controversial chnage to which multiple= > FreeBSD people said they were not sure what it is good for and then we > get panic reports from the same source about their own code? >=20 > I think it is time to backout the original at this point.=20 This panic is already fixed, and I added a test case to exercise (and illust= rate) the relevant code path.=20 At this point I see no reason to revert, especially as the revert is non-tri= vial. As discussed with secteam as well.=20 =E2=80=94=20 Kristof=