Fix udp_output() locking strategy in one case

[bz]

Summary

Fix udp_output() lock inconsistency.

In r297225 the initial INP_RLOCK() was replaced by an early
acquisition of an r- or w-lock depending on input variables
possibly extending the write locked area for reasons not entirely
clear but possibly to avoid a later case of unlock and relock
leading to a possible race condition and possibly in order to
allow the route cache to work for connected sockets.

Unfortunately the conditions were not 1:1 replicated (probably
because of the route cache needs). While this would not be a
problem the legacy IP code compared to IPv6 has an extra case
when dealing with IP_SENDSRCADDR. In a particular case we were
holding an exclusive inp lock and acquired the shared udbinfo
lock (now epoch).
When then running into an error case, the locking assertions
on release fired as the udpinfo and inp lock levels did not match.

Break up the special case and in that particular case acquire
and udpinfo lock depending on the exclusitivity of the inp lock.

MFC After: 1 week
Reported-by: [e-mail]

Original report can be found here:
https://groups.google.com/d/msg/syzkaller-freebsd-bugs/MHUQhKBFhHA/kBsbeOy-AwAJ

Test Plan

NOT TESTED AT ALL by me, @tuexen said he did; thank you!

[emaste]

Original commit Phabricator link rS297225

[emaste]

sys/netinet/udp_usrreq.c

1275 ↗

(On Diff #55107)

I think this comment does not reduce confusion. Is the statement that hash locking update is not correct and that the entire block comment above is wrong?

[tuexen]

I tested that this patch fixes the issue reported by syzkaller.

sys/netinet/udp_usrreq.c

1280 ↗

(On Diff #55107)

No code above this changes addr or sin. So I would just remove the duplicated assignment instead of changing it to a KASSERT.

[• karels]

sys/netinet/udp_usrreq.c

1292 ↗

(On Diff #55107)

I haven't thought this all the way through, but should this block be earlier, after line 1284? I.e. can both tests be true; if so, maybe we need the write lock?

[bz]

@emaste update the comment a bit.
@tuexen removed the KASSERT and left the sin = to be removed in a separate commit (as it is noise here)
@karels I think with the change we are fine as splitting up the case was for IP_SENDSRCADDR.