Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net: dummy byte in WriteMsgUnix breaks unixgram protocols #6476

Closed
alberts opened this issue Sep 25, 2013 · 7 comments

Comments

Projects
None yet
4 participants
@alberts
Copy link
Contributor

commented Sep 25, 2013

What steps will reproduce the problem?

https://groups.google.com/d/topic/golang-nuts/PyOeimgyvFw/discussion

WriteMsgUnix always sends a dummy normal byte if one tries to send only
out-of-band/ancillary data (e.g. a file descriptor), because ReadMsgUnix doesn't receive
oob-only writes (it just blocks) on stream sockets.

This was done based on prior art:

https://git.gitorious.org/libancillary/libancillary.git

http://svn.python.org/projects/python/branches/release32-maint/Modules/_multiprocessing/multiprocessing.c

and discussions:

https://groups.google.com/d/msg/golang-dev/Rkwgwug2OpI/7YYunK4Z9NUJ

https://golang.org/cl/2331044#msg12

However, it turns out that you can send oob-only messages on a datagram socket.

systemd implements a datagram socket protocol for talking to its journal to exchange
file descriptors of temporary files containing messages that are too long to send via
the datagram socket directly.

systemd doesn't expect the dummy byte and returns an error if it is present.

Removing the dummy byte will cause this test with a stream socket to hang:

http://play.golang.org/p/ebLI7mwBPF

but this test with a datagram socket still works:

http://play.golang.org/p/q-eWEP3_96

which is expected, because systemd has a test for this scenario.

Which version are you using?  (run 'go version')

tip

Please provide any additional information below.

systemd code:

http://cgit.freedesktop.org/systemd/systemd/tree/src/journal/journal-send.c#n345
http://cgit.freedesktop.org/systemd/systemd/tree/src/journal/journald-server.c#n1197
http://cgit.freedesktop.org/systemd/systemd/tree/src/journal/test-journal-send.c#n48

systemd test output shows that the test passes:

https://gist.github.com/philips/1938c88d544e49f40889/raw/cf64b66c261aee63c0476b28a32e448ecb2d5b60/gistfile1.txt
@alberts

This comment has been minimized.

Copy link
Contributor Author

commented Sep 25, 2013

Comment 1:

It's probably worth dropping the dummy byte for the datagram case.
@alberts

This comment has been minimized.

Copy link
Contributor Author

commented Sep 25, 2013

Comment 2:

Or one might argue that the standard library has no business ever adding a dummy byte.
One might consider panicking if anyone does WriteMsgUnix(nil, oob), but maybe it works
if one does:
WriteMsgUnix(data, nil)
WriteMsgUnix(nil, oob)
WriteMsgUnix(data, nil)
on a stream socket. Need to check that.
@minux

This comment has been minimized.

Copy link
Member

commented Sep 25, 2013

Comment 3:

I guess we can't make the WriteMsgUnix(nil, oob) panic, but we probably
should address the no in-band data in the datagram case.
@mikioh, please triage this issue.

Labels changed: added go1.2maybe.

Status changed to Accepted.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Sep 26, 2013

Comment 4:

Not for Go 1.2.

Labels changed: added priority-later, go1.3maybe, removed priority-triage, go1.2maybe.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Dec 4, 2013

Comment 5:

Labels changed: added release-none, removed go1.3maybe.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Dec 4, 2013

Comment 6:

Labels changed: added repo-main.

@gopherbot

This comment has been minimized.

Copy link

commented Jun 15, 2017

CL https://golang.org/cl/45872 mentions this issue.

@gopherbot gopherbot closed this in 93da0b6 Aug 29, 2017

@golang golang locked and limited conversation to collaborators Aug 29, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.