Page MenuHomeFreeBSD

divert: Fix mbuf ownership confusion in div_output()
ClosedPublic

Authored by markj on May 5 2021, 6:20 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Dec 3, 11:57 AM
Unknown Object (File)
Oct 5 2024, 7:41 PM
Unknown Object (File)
Oct 5 2024, 4:57 AM
Unknown Object (File)
Oct 4 2024, 4:34 AM
Unknown Object (File)
Oct 3 2024, 12:43 PM
Unknown Object (File)
Oct 3 2024, 9:44 AM
Unknown Object (File)
Oct 2 2024, 10:13 PM
Unknown Object (File)
Oct 2 2024, 7:51 PM

Details

Summary

div_output_outbound() and div_output_inbound() relied on the caller to
free the mbuf if an error occurred. However, this is contrary to the
semantics of their callees, ip_output(), ip6_output() and
netisr_queue_src(), which always consume the mbuf. So, if one of these
functions returned an error, that would get propagated up to
div_output(), resulting in a double free.

Fix the problem by making div_output_outbound() and div_output_inbound()
responsible for freeing the mbuf in all cases.

Diff Detail

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

Event Timeline

markj requested review of this revision.May 5 2021, 6:20 PM
donner added inline comments.
sys/netinet/ip_divert.c
424

Should the comment also be changed?

508

Does ip_output always consume the mbuf, even in the case of an error?

514

Does ip6_output always consume the mbuf, even in the case of an error?

markj marked an inline comment as done.May 5 2021, 6:33 PM
markj added inline comments.
sys/netinet/ip_divert.c
508

Yes. If it succeeds, it hands the mbuf to lower layers, which must dispose of it in some way. Ditto for ip6_output().

Update the comment for div_output_outbound() like I did for
div_output_inbound().

Okay, the ip*_output functions will always consume the mbuf. I just read the souce

This revision is now accepted and ready to land.May 5 2021, 6:44 PM
avg removed a subscriber: avg.