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)
Feb 23 2024, 2:01 AM
Unknown Object (File)
Feb 14 2024, 4:55 AM
Unknown Object (File)
Dec 20 2023, 5:28 AM
Unknown Object (File)
Dec 13 2023, 12:21 PM
Unknown Object (File)
Oct 6 2023, 3:48 PM
Unknown Object (File)
Sep 6 2023, 12:30 AM
Unknown Object (File)
Sep 3 2023, 2:34 AM
Unknown Object (File)
Sep 3 2023, 2:33 AM

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
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 39034
Build 35923: arc lint + arc unit

Event Timeline

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

Should the comment also be changed?

509

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

515

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
509

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.