> On 4 Sep 2022, at 16:29, Vitaliy Makkoveev <o...@bsdbox.dev> wrote:
>
>> On 4 Sep 2022, at 12:30, Klemens Nanni <k...@openbsd.org> wrote:
>>
>>> The diff has been committed but the problem remains:
>>>
>>> OpenBSD 7.2-beta (GENERIC.MP) #2: Thu Sep 1 18:54:34 CEST 2022
>>>
>>> s...@bev.stsp.name:/usr/src/sys/arch/amd64/compile/GENERIC.MP
>>>
>>> login: kernel: protection fault trap, code=0
>>> Stopped at rt_ifa_del+0x39: movb 0x1b6(%rax),%bl
>>> ddb{3}> bt
>>> rt_ifa_del(ffff800000496c00,800100,dead0009dead4110,0) at rt_ifa_del+0x39
>>> in6_unlink_ifa(ffff800000496c00,ffff8000000da2a8) at in6_unlink_ifa+0xae
>>> in6_purgeaddr(ffff800000496c00) at in6_purgeaddr+0x127
>>> nd6_expire(0) at nd6_expire+0x96
>>> taskq_thread(ffff80000002c080) at taskq_thread+0x100
>>> end trace frame: 0x0, count: -5
>>> ddb{3}> show struct ifaddr 0xffff800000496c00
>>> struct ifaddr at 0xffff800000496c00 (64 bytes) {ifa_addr = (struct sockaddr
>>> *)0
>>> xdead0009dead4110, ifa_dstaddr = (struct sockaddr *)0x4002e6f6e3c87f50,
>>> ifa_net
>>> mask = (struct sockaddr *)0xdead4110dead4110, ifa_ifp = (struct ifnet
>>> *)0xdead4
>>> 110dead4110, ifa_list = {tqe_next = (struct ifaddr *)0xdead4110dead4110,
>>> tqe_pr
>>> ev = 0xdead4110dead4110}, ifa_flags = 0xdead4110, ifa_refcnt = {r_refs =
>>> 0xdead
>>> 4110, r_traceidx = 0xdead4110}, ifa_metric = 0xdead4110}
>>> ddb{3}>
>>>
>>
>> Glancing at nd6_expire()... does this diff help?
>
> The `ifa’ already has the reference for the `ifa_list’, no need to bump it.
>
> It looks like we have lockless modification access to to the `ifa_list’ or
> we missed ifa_del() before ifafree().
>
>>
>> Index: sys/netinet6/nd6.c
>> ===================================================================
>> RCS file: /cvs/src/sys/netinet6/nd6.c,v
>> retrieving revision 1.246
>> diff -u -p -r1.246 nd6.c
>> --- sys/netinet6/nd6.c 9 Aug 2022 21:10:03 -0000 1.246
>> +++ sys/netinet6/nd6.c 4 Sep 2022 09:26:15 -0000
>> @@ -496,7 +496,7 @@ nd6_expire(void *unused)
>> TAILQ_FOREACH_SAFE(ifa, &ifp->if_addrlist, ifa_list, nifa) {
>> if (ifa->ifa_addr->sa_family != AF_INET6)
>> continue;
>> - ia6 = ifatoia6(ifa);
>> + ia6 = ifatoia6(ifaref(ifa));
>> /* check address lifetime */
>> if (IFA6_IS_INVALID(ia6)) {
>> in6_purgeaddr(&ia6->ia_ifa);
In addition, you forgot to release the reference.
