On Thu, Jun 25, 2020 at 2:49 AM Martin Pieuchot <[email protected]> wrote: > > On 24/06/20(Wed) 19:54, Jason A. Donenfeld wrote: > > On Wed, Jun 24, 2020 at 4:02 AM Martin Pieuchot <[email protected]> wrote: > > > Yes, that might be a better way. If I understood your original mail the > > > issue is related to ifunit(), right? ifunit() is not used in packet- > > > processing contexts, that's why we did not protect it by the NET_LOCK(). > > > > > > I'm not sure if all the ifunit() usages are necessary, many of them are > > > certainly exposing races with attach/destroy. > > > > No, not _just_ ifunit, but ifunit is one of the places where this is > > hit. But just one. > > Which ones then? I'd be delighted if you could share those facts.
You make it sound as if I'm deliberately concealing it in order to dangle a tasty orange in front of you, but that's not what I meant. Just look around in the source code at all of the code that uses an ifp outside of a reference count or other locking semantics. Grepping around, for example, I found ifioctl->ifioctl_get->ifconf->if_list->kaboom. There's lots of interesting behavior in there, and a pretty good indication that you really don't want ioctls racing with either clone or destroy, which is what my patch fixes.
