Fix a race in udp6_output

[tuexen]

Summary

r348494 fixes a race in udp_output(). The same race exists in udp_output6() and was discovered by syzkaller. Therefore apply a similar patch to IPv6.

[ae]

sys/netinet6/udp6_usrreq.c

757 ↗

(On Diff #59678)

It seems it is not possible for sin6 become not NULL here. Maybe it would be simpler to add goto to the "else {}" branch of this condition?

[ae]

sys/netinet6/udp6_usrreq.c

757 ↗

(On Diff #59678)

I mean sin6 can not become NULL

[tuexen]

sys/netinet6/udp6_usrreq.c

757 ↗

(On Diff #59678)

If sin6 is NULL it will stay so, and there is no need to retry. However, if sin6 is not NULL and the inp based condition changed after getting the inp lock, we need to retry. So I think the condition is correct. Am I missing something?

[ae]

sys/netinet6/udp6_usrreq.c

757 ↗

(On Diff #59678)

Yes, I'm agree that condition is correct, I'm just a bit confused, why we need to make the check three times.
Also, it seems that bound socket can not have inp_lport = 0, so may be it is enough to only check inp_lport?

[markj]

This matches the IPv4 change, so I'm fine with it going in. I agree that having a goto to the read lock case would make a bit more sense.

[tuexen]

sys/netinet6/udp6_usrreq.c

757 ↗

(On Diff #59678)

I agree with you that the code might not be optimal. But the intend of this fix is to fix a bug FreeBSD had in the IPv4 and IPv6 code path and that was fixed in the IPv4 code path, but not in the IPv6 code path. So I would like to fix it here in a consistent way.

Improving the code could be done in a separate commit, doing similar changes to the IPv4 and IV6 code to keep it as consistent as possible.
ae@: OK?

[bz]

Thanks, was still on my TODO. Always happy if someone does the work for me :-)