On Tue, 30 Sep 2025, Zhenlei Huang wrote:
On Sep 30, 2025, at 10:59 AM, Mateusz Guzik <[email protected]> wrote:
On Tue, Sep 30, 2025 at 4:38 AM Zhenlei Huang <[email protected]
<mailto:[email protected]>> wrote:
On Sep 29, 2025, at 10:29 PM, Mateusz Guzik <[email protected]> wrote:
On Tue, Oct 15, 2013 at 12:31 PM Gleb Smirnoff <[email protected]> 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;
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=9e792f7ef7298080c058fbc2d36a4e60e596dae9
.
See https://reviews.freebsd.org/D51778 for more context.
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?
I think it is time to backout the original at this point. At least then
the commit history can be fixed?
/bz
--
Bjoern A. Zeeb r15:7
