Summary: | [socket] FD leak by receiving SCM_RIGHTS by recvmsg with small control message buffer | ||
---|---|---|---|
Product: | Base System | Reporter: | Tanaka Akira <akr> |
Component: | kern | Assignee: | Mark Johnston <markj> |
Status: | Closed FIXED | ||
Severity: | Affects Only Me | CC: | crest, koobs, markj, net, pho |
Priority: | Normal | Flags: | koobs:
mfc-stable11+
|
Version: | 11.0-RELEASE | ||
Hardware: | Any | ||
OS: | Any |
Description
Tanaka Akira
2009-02-19 15:20:00 UTC
Responsible Changed From-To: freebsd-bugs->rwatson Take ownership of this as I've been spending some time with UNIX domain socket sockets. I just ran into this very bug on FreeBSD 11.0p7. It looks to me as though the loop in kern_recvit() which sets MSG_CTRUNC needs to use dom_dispose to ensure that the remaining rights are closed before the mbuf is freed. Unfortunately, dom_dispose wants a socket rather than an mbuf - this was changed in r285522 to fix a race between unp_dispose() and unp_gc(). However, in this case the truncated mbuf will have already been removed from the socket buffer, so we don't need to worry about the race. (If MSG_PEEK is set, the rights will still be present in the sockbuf and we needn't do anything.) So, to fix this we'd either need a way to invoke dom_dispose on a plain mbuf (i.e., the KPI prior to r285522), or we need to fake up a socket like sorflush() does. A commit references this bug: Author: markj Date: Sun Jul 22 18:07:09 UTC 2018 New revision: 336614 URL: https://svnweb.freebsd.org/changeset/base/336614 Log: Add a regression test for PR 131876. PR: 131876 MFC after: 1 week Changes: head/tests/sys/kern/unix_passfd_test.c A commit references this bug: Author: markj Date: Sun Jul 29 19:12:06 UTC 2018 New revision: 336873 URL: https://svnweb.freebsd.org/changeset/base/336873 Log: MFC r336614: Add a regression test for PR 131876. PR: 131876 Changes: _U stable/11/ stable/11/tests/sys/kern/unix_passfd_test.c A commit references this bug: Author: markj Date: Tue Jul 31 00:48:08 UTC 2018 New revision: 336957 URL: https://svnweb.freebsd.org/changeset/base/336957 Log: Add a regression test related to PR 131876. If an error occurs while copying a SCM_RIGHTS message to userspace, we free the mbuf containing externalized rights, leaking them. PR: 131876 MFC after: 1 week Sponsored by: The FreeBSD Foundation Changes: head/tests/sys/kern/unix_passfd_test.c A commit references this bug: Author: markj Date: Tue Aug 7 15:04:53 UTC 2018 New revision: 337421 URL: https://svnweb.freebsd.org/changeset/base/337421 Log: MFC r336957: Add a regression test related to PR 131876. PR: 131876 Changes: _U stable/11/ stable/11/tests/sys/kern/unix_passfd_test.c A commit references this bug: Author: markj Date: Tue Aug 7 16:36:50 UTC 2018 New revision: 337423 URL: https://svnweb.freebsd.org/changeset/base/337423 Log: Improve handling of control message truncation. If a recvmsg(2) or recvmmsg(2) caller doesn't provide sufficient space for all control messages, the kernel sets MSG_CTRUNC in the message flags to indicate truncation of the control messages. In the case of SCM_RIGHTS messages, however, we were failing to dispose of the rights that had already been externalized into the recipient's file descriptor table. Add a new function and mbuf type to handle this cleanup task, and use it any time we fail to copy control messages out to the recipient. To simplify cleanup, control message truncation is now only performed at control message boundaries. The change also fixes a few related bugs: - Rights could be leaked to the recipient process if an error occurred while copying out a message's contents. - We failed to set MSG_CTRUNC if the truncation occurred on a control message boundary, e.g., if the caller received two control messages and provided only the exact amount of buffer space needed for the first. PR: 131876 Reviewed by: ed (previous version) MFC after: 1 month Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D16561 Changes: head/sys/compat/cloudabi/cloudabi_sock.c head/sys/compat/freebsd32/freebsd32_misc.c head/sys/compat/linux/linux_socket.c head/sys/kern/uipc_syscalls.c head/sys/kern/uipc_usrreq.c head/sys/sys/mbuf.h A commit references this bug: Author: markj Date: Tue Aug 7 16:39:08 UTC 2018 New revision: 337424 URL: https://svnweb.freebsd.org/changeset/base/337424 Log: Update PR 131876 regression tests after r337423. - Add some more cases to the truncation test. - Remove the "expect fail" annotations. PR: 131876 MFC after: 1 month Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D16562 Changes: head/tests/sys/kern/unix_passfd_test.c A commit references this bug: Author: markj Date: Wed Sep 12 19:13:34 UTC 2018 New revision: 338618 URL: https://svnweb.freebsd.org/changeset/base/338618 Log: MFC r337423: Improve handling of control message truncation. PR: 131876 Changes: _U stable/11/ stable/11/sys/compat/cloudabi/cloudabi_sock.c stable/11/sys/compat/freebsd32/freebsd32_misc.c stable/11/sys/compat/linux/linux_socket.c stable/11/sys/kern/uipc_syscalls.c stable/11/sys/kern/uipc_usrreq.c stable/11/sys/sys/mbuf.h A commit references this bug: Author: markj Date: Wed Sep 12 19:15:58 UTC 2018 New revision: 338619 URL: https://svnweb.freebsd.org/changeset/base/338619 Log: MFC r337329: Fix the regression test for PR 181741. MFC r337424: Update PR 131876 regression tests after r337423. PR: 131876 Changes: _U stable/11/ stable/11/tests/sys/kern/unix_passfd_test.c Correct version (latest reported in) and MFC status I still see a leak with this test scenario after running it multiple times: https://people.freebsd.org/~pho/stress/log/sendmsg2.txt on FreeBSD 13.0-CURRENT #0 r343687 (In reply to Peter Holm from comment #14) Hmm, this doesn't appear to be an fd leak but rather a refcount leak on the unp pcb. (In reply to Peter Holm from comment #14) This was fixed by r343784, which has been merged to the affected branches. |