netinet6/nd6: Sprinkle missing prefix refcounting
ClosedPublic
Actions

Authored by markj on Thu, Jul 23, 6:59 PM.

Details

Reviewers

pouria
zlei

Group Reviewers

network

Commits

rG76ca489e0f14: netinet6/nd6: Sprinkle missing prefix refcounting

Summary

When we drop the prefix lock to call nd6_prefix_offlink() or
nd6_prefix_onlink(), make sure to keep the correpsonding prefix
structure alive. It is possible for a concurrent nd6_timer() to expire
the prefix while the lock is dropped.

Reported by: Maik Muench of Secfault Security

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 75056
Build 71939: arc lint + arc unit

Event Timeline

markj created this revision.Thu, Jul 23, 6:59 PM

Herald added subscribers: pouria, glebius, melifaro and 2 others. · View Herald TranscriptThu, Jul 23, 6:59 PM

markj requested review of this revision.Thu, Jul 23, 6:59 PM

Harbormaster completed remote builds in B75056: Diff 182553.Thu, Jul 23, 7:00 PM

pouria added inline comments.Thu, Jul 23, 7:08 PM

sys/netinet6/nd6_rtr.c
1962–1963	shouldn't be after rlock?
2237–2238	same here

pouria added inline comments.Thu, Jul 23, 7:14 PM

sys/netinet6/nd6_rtr.c
1962–1963	shouldn't be after rlock? We don't call it after RLOCK. So it should be fine there.
2237–2238	same here Maybe even after a_failure. Because we unlock it again and try to dereference it after.

markj added inline comments.Thu, Jul 23, 7:51 PM

sys/netinet6/nd6_rtr.c
1962–1963	There is no reason to acquire it after the read lock is reacquired.
2237–2238	I'm not sure what you are suggesting with a_failure. Note, if the prefix was unlinked, then V_nd6_list_genid will be incremented. Conversely, if V_nd6_list_genid wasn't incremented, then the prefix must still be linked into the prefix list, so it's safe to use it to continue iterating.

pouria added inline comments.Thu, Jul 23, 7:59 PM

sys/netinet6/nd6_rtr.c
2254	I'm talking about here. If I understand it correctly: Consider we unlock above again and we trying to dereference pr here. So if timer runs right after above unlock and expire our pr, same thing could happen, right? Or am I missing something?