Page MenuHomeFreeBSD

Update udp6_output() inp locking to avoid concurrency issues with route cache updates
ClosedPublic

Authored by bz on Sep 18 2018, 10:26 PM.

Details

Summary

Bring over locking changes applied to udp_output() for the route cache in r297225
and fixed in r306559 which achieve multiple things:
(1) acquire an exclusive inp lock earlier depending on the expected conditions; we add a comment explaining this in udp6,
(2) having acquired the exclusive lock earlier eliminates a slight possible chance for a race condition which was present in v4 for multiple years as well and is now gone, and
(3) only pass the inp_route6 to ip6_output() if we are holding an exclusive inp lock, so possible route cache updates in case of routing table generation number changes can happen savely.
In addition this change (as the legacy IP counterpart) decomposes the tracking
of inp and pcbinfo lock and adds extra assertions, that the two together are
acquired correctly.

PR: 230950
Pointyhat to: bz (for completely missing this bit)

Test Plan

I wrote and independent test case, which I'll convert to atf and commit
seperately.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

bz added a reviewer: emaste.

It would be good to see if the crashes stop before committing this, but it looks right to me.

sys/netinet6/udp6_usrreq.c
742 ↗(On Diff #48194)

nit: should "on" be "or"?

This revision is now accepted and ready to land.Sep 19 2018, 12:00 AM
markj added inline comments.
sys/netinet6/udp6_usrreq.c
701 ↗(On Diff #48194)

Style: unlock_inp should come first.

Maybe add the macro to deduplicate the code? It repeats 4 times...

#define INP_COND_UNLOCK(inp, locked)   \
    if ((locked) == UH_WLOCKED)
        INP_WUNLOCK(inp);
    else
        INP_RUNLOCK(inp);
This revision was automatically updated to reflect the committed changes.
bz marked an inline comment as done.