|Summary:||Passed FDs are incorrectly assigned to messages|
|Product:||dbus||Reporter:||David Herrmann <dh.herrmann>|
|Component:||core||Assignee:||D-Bus Maintainers <dbus>|
|Status:||RESOLVED MOVED||QA Contact:||D-Bus Maintainers <dbus>|
|Priority:||medium||CC:||dh.herrmann, lennart, teg|
|i915 platform:||i915 features:|
Description David Herrmann 2017-07-11 12:48:52 UTC
Hi The dbus specification states: `UNIX_FDS: The number of Unix file descriptors that accompany the message. If omitted, it is assumed that no Unix file descriptors accompany the message. The actual file descriptors need to be transferred via platform specific mechanism out-of-band. They must be sent at the same time as part of the message itself. They may not be sent before the first byte of the message itself is transferred or after the last byte of the message itself.` The spec clearly states that FDs must be sent together with the actual bytes of the message they belong to. However, dbus-daemon does not verify that behavior. In particular, if you send a message with 1 FD, but UNIX_FDS set to 0, followed by a message with 0 FDs, but UNIX_FDS set to 1, then dbus-daemon will attribute that FD to the second message. It is not clear whether dbus-daemon behaves incorrectly here. The spec only defines how *senders* are supposed to work. However, it is very unfortunate that dbus-daemon does not verify that behavior. In particular, either the behavior of dbus-daemon is wanted, in which case the spec is needlessly restricted, or the definition of the spec is what is wanted, then dbus-daemon needs to be fixed. IOW: Right now, the spec treats the stream of bytes and FDs to be the same stream. But dbus-daemon treats both as two independent streams, where the byte-stream controls when to pop-off elements from the FD stream. It would be nice if we can clear this up. Thanks David
Comment 1 Simon McVittie 2017-07-18 13:50:20 UTC
Lennart, you implemented fd-passing if I remember correctly: do you have an opinion here? I would tend to believe the spec on this, not the implementation. Presumably to make the dbus-daemon validate, we would have to store (fd, offset) pairs in the message reader instead of fds, and when we reconstruct a message, include all fds from offsets less than its length, then shift down all the remaining offsets by the appropriate amount at the same time we memmove() the rest of the buffer down ready for the next message? I know Linux probably doesn't combine sent AF_UNIX buffers that have ancillary data into the same recvmsg() as prior buffers (in particular the tail of the previous message); but I don't think we can *necessarily* rely on other kernels doing the same, so it's probably safest not to assume this.
Comment 2 David Herrmann 2017-07-31 12:15:37 UTC
(In reply to Simon McVittie from comment #1) > I know Linux probably doesn't combine sent AF_UNIX buffers that have > ancillary data into the same recvmsg() as prior buffers (in particular the > tail of the previous message); but I don't think we can *necessarily* rely > on other kernels doing the same, so it's probably safest not to assume this. On linux, a stream socket is internally a linked list of socket-buffers (skbs). A single recvmsg(2) call can return combined information from many skbs. Hence, the internal fragmentation across skbs is not observable from user-space (mostly..). Furthermore, a single sendmsg(2) call will always allocate its own skb, rather than merging them into a possible pending previous skb that has room left. Lastly, a single sendmsg(2) call allocates exactly 1 skb, possibly ending in a short write if the request is too big. Now, when we consider ancillary data this behavior changes slightly. First of all, ancillary data is always attached to an skb without any further positional index. A single sendmsg(2) call always consumes all provided ancillary data and attaches it to the allocated skb. On the other side, recvmsg(2) returns short reads *after* any skb with ancillary data (except for UCREDS which can be merged if equivalent across skbs). This means, when you call recvmsg(2), the ancillary data returned belongs to the *LAST* byte in that blob. The call might have merged many skbs without ancillary data up to a last skb which has ancillary data. Since it is impossible to deduce the skb-merges afterwards, the ancillary data must be attributed to the last byte, and as such to the message that this last byte belongs to. Furthermore, if we follow the dbus-spec, all ancillary data must be sent *AT ONCE* after the first byte of a message, and before the last byte (including). Hence, a receiver must treat a message as invalid if it ends up receiving more than one set of ancillary data with a message. The dbus-broker input-queue implements this logic , and we are not aware of any issues so far.  https://github.com/bus1/dbus-broker/blob/master/src/dbus/queue.c#L324
Comment 3 GitLab Migration User 2018-10-12 21:31:29 UTC
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/dbus/dbus/issues/183.