Page MenuHomeFreeBSD

Add ifa_try_ref() to simplify ifa handling inside epoch.
ClosedPublic

Authored by melifaro on Feb 13 2021, 12:21 AM.
Tags
None
Referenced Files
F102547401: D28639.id83804.diff
Wed, Nov 13, 9:55 PM
Unknown Object (File)
Tue, Nov 5, 1:40 PM
Unknown Object (File)
Fri, Oct 18, 7:50 PM
Unknown Object (File)
Sep 24 2024, 1:25 PM
Unknown Object (File)
Sep 24 2024, 11:19 AM
Unknown Object (File)
Sep 14 2024, 5:38 PM
Unknown Object (File)
Sep 5 2024, 9:39 PM
Unknown Object (File)
Sep 5 2024, 3:29 PM
Subscribers

Details

Summary

More and more code migrates from lock-based protection to the NET_EPOCH umbrella.
It requires some logic changes, including, notably, refcount handling.

When we have an ifa pointer and we're running inside epoch we're guaranteed that this pointer will not be freed.
However, the following case can still happen:

  • 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

To address it, ifa_try_ref() function is added, allowing to return failure when we try to reference ifa with 0 refcount.
Additionally, existing ifa_ref() is enforced with KASSERT to provide cleaner error in such scenarious.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

melifaro added a reviewer: network.

Is the destroy callout guaranteed to run outside (or on the border) of epoch? Can this callout check the refcount and chase to abort itself?

Is the destroy callout guaranteed to run outside (or on the border) of epoch? Can this callout check the refcount and chase to abort itself?

Destroy callout runs outside the current epoch. Callout can check the refcount value and prevent _direct_ use-after-free.

There is a potential problem with this approach.
As ifa refcount==0 means that it has been deleted from the lists, we're adding "orphan" ifa, which, by the way, still has ifp pointer.
It may complicate ifp/ifa teardown procedure by allowing to endlessly refcount half-dead ifa pointer.
IMO it is always easier to think and operate with object lifecycle model which has a clear "dying" step which is a one-way door towards the destruction.

This revision is now accepted and ready to land.Feb 16 2021, 7:55 PM