Date: Fri, 12 Feb 2021 16:48:12 -0500 From: Ryan Stone <rysto32@gmail.com> To: "Alexander V. Chernikov" <melifaro@freebsd.org> Cc: freebsd-net <freebsd-net@freebsd.org> Subject: Re: ifaddr reference count leaks that seem to be related to routing code Message-ID: <CAFMmRNyq2qEkjGw7i7sD=FghorEwX%2Br6VHsqnhhMPF3rj9bp9Q@mail.gmail.com> In-Reply-To: <1464101613162719@mail.yandex.ru> References: <CAFMmRNyi3bXmJd%2BHrMAOSuB2cm%2B5ZtC5jaj3jLiHs0RVMBegZA@mail.gmail.com> <113921613153727@vla1-bff2c05bedc2.qloud-c.yandex.net> <CAFMmRNw7i=aD2JXetnaPSBzirSxwvAq6ZzbwQ1_=qKyZMc50%2BA@mail.gmail.com> <1464101613162719@mail.yandex.ru>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Feb 12, 2021 at 4:14 PM Alexander V. Chernikov <melifaro@freebsd.org> wrote: > The slightly different approach here: https://reviews.freebsd.org/D28629 > We indeed are running under epoch, so that prevents _immediate_ ifa destruction. > However, we still can run into the situation when > * in thread 1 we drop to 0 refcount for ifa and schedule its deletion. > * in thread 2 we use this ifa and reference it > * destroy callout kicks in > * unhappy user reports bug > The current approach minimises this possibility by taking an ifa refcount early. > More general approach would probably be in introducing `ifa_try_ref()` based on refcount_acquire_if_not_zero() but that's something that needs a bit more effort. Oops, you're completely correct. Do you think that we should put a KASSERT in ifa_ref() that ifa_refcnt > 0 to catch such a bug if somebody ever manages to introduce one? Of course it would not fire except in the case where we already lost the race, but at least it would make it easier to debug.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAFMmRNyq2qEkjGw7i7sD=FghorEwX%2Br6VHsqnhhMPF3rj9bp9Q>