Page MenuHomeFreeBSD

Free rights in truncated control messages.
ClosedPublic

Authored by markj on Aug 1 2018, 11:38 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Mar 10, 3:38 AM
Unknown Object (File)
Sun, Mar 10, 3:38 AM
Unknown Object (File)
Sun, Mar 10, 3:38 AM
Unknown Object (File)
Sun, Mar 10, 3:38 AM
Unknown Object (File)
Sun, Mar 10, 3:26 AM
Unknown Object (File)
Thu, Feb 22, 11:42 PM
Unknown Object (File)
Feb 5 2024, 10:00 PM
Unknown Object (File)
Jan 22 2024, 5:05 PM
Subscribers

Details

Summary

Control messages may be lost in some cases: if copyout() fails, or if
the receiving process does not supply sufficient buffer space. In the
process of externalizing these control messages, their data may now
reference kernel resources; if these references cannot be transferred to
the recipient, they must be properly disposed of. This occurs
specifically when transmitting SCM_RIGHTS messages over a unix socket,
and results in a file descriptor leak when the control message mbuf
chain is freed.

Fix the problem using a new function, m_dispose_extcontrolm(), which is
responsible for releasing any resources contained in an externalized
control message. Such messages are stored in mbufs with a distinguished
type; once the resources are disposed, the type changes back to
MT_CONTROL. Right now, the only source of MT_EXTCONTROL messages will
use a separate mbuf for each control message, but
m_dispose_extcontrolm() attempts to handle multiple control messages in
a single mbuf.

If an externalized message would be truncated part way through, drop the
whole message. This makes disposal much simpler and follows NetBSD's
behaviour. I can't see any reason to try a partial copy as we do today,
and it would be tricky to support that.

Also fix a related bug in kern_recvit(): if truncation happens at an
mbuf boundary, we were failing to set MSG_CTRUNC.

Test Plan

Regression tests in D16562.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

markj added reviewers: ed, network.

Looks good. Thanks a lot for working on this!

That said, I'm never a true expert when it comes to our network stack. Maybe best if some in addition to me reviews the changes to sys/kern/uipc_syscalls.c?

sys/compat/freebsd32/freebsd32_misc.c
1197 ↗(On Diff #46166)

Nit: extraneous parentheses.

This revision is now accepted and ready to land.Aug 2 2018, 6:13 AM
markj marked an inline comment as done.Aug 2 2018, 2:30 PM
In D16561#351555, @ed wrote:

Looks good. Thanks a lot for working on this!

That said, I'm never a true expert when it comes to our network stack. Maybe best if some in addition to me reviews the changes to sys/kern/uipc_syscalls.c?

Hopefully someone from network will chime in. :) I just wanted to make sure you were happy with the cloudabi changes.

sys/sys/mbuf.h
637 ↗(On Diff #46166)

This header is probably the wrong place for a function defined in uipc_syscalls.c, but I'm not sure where it would logically go. Suggestions are welcome.

Fix bugs in freebsd32.

  • Set MSG_CTRUNC if truncation occurs at a control message boundary.
  • Only update cmsg_len when copying out to avoid later confusion in m_dispose_extcontrolm().
This revision now requires review to proceed.Aug 6 2018, 8:05 PM
This revision was not accepted when it landed; it landed in state Needs Review.Aug 7 2018, 4:36 PM
This revision was automatically updated to reflect the committed changes.