Backport #7786: fix for abstract namespace

• Status: Closed
• Assignee: usa (Usaku NAKAMURA)

[shugo (Shugo Maeda)]

=begin
Please apply the attached patch to Ruby 1.9.3.

In Ruby 1.9.3, the addrlen argument for bind(2) and connect(2) is set to sizeof(struct sockaddr_un).
However, on Linux, if sun_path starts with NUL, which means that the socket's address is in abstract namespace, the first (addrlen - sizeof(sa_family_t)) bytes of sun_path is used as the name of the socket, so the socket name is filled with extra NULs unintentionally. Please see unix(7) for details.

The patch introduces a new function rsock_unix_sockaddr_len() to calculate the correct address length on Linux.

Besides that, the patch includes the following changes.

• fix Socket.unix_server_socket not to access the file system if the specified name is in abstract namespace.
• fix rsock_init_unixsock to use rb_inspect() to avoid ArgumentError in rb_sys_fail_str() when path includes NUL.
• support the longest path in sockaddr_un.
  • This change is necessary for backward compatibility. Without it, the old name filled with extra NULs cannot be specified like UNIXSocket.open(name + "\0" * (108 - name.bytesize)).
    =end

[usa (Usaku NAKAMURA)]

Q1: Is this related to [Backport #7775] ?
Q2: Can you show the revision of the related change(s) in trunk?
Q3: Where the "attached patch"? :)

[shugo (Shugo Maeda)]

• File abstract_namespace_fix.diff abstract_namespace_fix.diff added

usa (Usaku NAKAMURA) wrote:

Q1: Is this related to [Backport #7775] ?

It's related, but independent.
#7775 is needed to receive the address of a socket correctly.
#7786 is needed to specify the address of a socket correctly.

Q2: Can you show the revision of the related change(s) in trunk?

r35474, r35479, r38939, r38943, r38963, r38991, and r38994.

Q3: Where the "attached patch"? :)

Sorry, I've attached it.

[akr (Akira Tanaka)]

2013/2/5 shugo (Shugo Maeda) [e-mail]:

• support the longest path in sockaddr_un.
  • This change is necessary for backward compatibility. Without it, the old name filled with extra NULs cannot be specified like UNIXSocket.open(name + "\0" * (108 - name.bytesize)).

I'm not sure about compatibility here.

If someone uses UNIXServer.open("\0foo") and UNIXSocket.open("\0foo") for
current Ruby, how they should be modified with compatible fasion?

Tanaka Akira

[shugo (Shugo Maeda)]

akr (Akira Tanaka) wrote:

2013/2/5 shugo (Shugo Maeda) [e-mail]:

• support the longest path in sockaddr_un.
  • This change is necessary for backward compatibility. Without it, the old name filled with extra NULs cannot be specified like UNIXSocket.open(name + "\0" * (108 - name.bytesize)).

I'm not sure about compatibility here.

If someone uses UNIXServer.open("\0foo") and UNIXSocket.open("\0foo") for
current Ruby, how they should be modified with compatible fasion?

The following code uses the name "\0foo\0\0\0....." even if the patch is applied.
require "socket"

OFFSET_OF_SUN_PATH = 2
UNIX_PATH_MAX = 108
SIZE_OF_SOCKADDR_UN = OFFSET_OF_SUN_PATH + UNIX_PATH_MAX
name = "\0foo"
begin
addr = Socket.pack_sockaddr_un(name)
if addr.bytesize < SIZE_OF_SOCKADDR_UN
# append extra NULs for broken abstract namespace handling.
sun_path = name + "\0" * (UNIX_PATH_MAX - name.bytesize)
else
sun_path = name
end
rescue ArgumentError
sun_path = name
end
UNIXSocket.open(sun_path) do
# ...
end

However, if the longest path in sockaddr_un is not supported, this code doesn't work.

[akr (Akira Tanaka)]

2013/2/6 shugo (Shugo Maeda) [e-mail]:

If someone uses UNIXServer.open("\0foo") and UNIXSocket.open("\0foo") for
current Ruby, how they should be modified with compatible fasion?

The following code uses the name "\0foo\0\0\0....." even if the patch is applied.
require "socket"

OFFSET_OF_SUN_PATH = 2
UNIX_PATH_MAX = 108
SIZE_OF_SOCKADDR_UN = OFFSET_OF_SUN_PATH + UNIX_PATH_MAX
name = "\0foo"
begin
addr = Socket.pack_sockaddr_un(name)
if addr.bytesize < SIZE_OF_SOCKADDR_UN
# append extra NULs for broken abstract namespace handling.
sun_path = name + "\0" * (UNIX_PATH_MAX - name.bytesize)
else
sun_path = name
end
rescue ArgumentError
sun_path = name
end
UNIXSocket.open(sun_path) do
# ...
end

However, if the longest path in sockaddr_un is not supported, this code doesn't work.

It seems not so trivial.

Also, is the code work well with Ruby 1.9.2 (or earlier Ruby 1.9.3)
and Ruby 2.0?

Tanaka Akira

[shugo (Shugo Maeda)]

akr (Akira Tanaka) wrote:

However, if the longest path in sockaddr_un is not supported, this code doesn't work.

It seems not so trivial.

Also, is the code work well with Ruby 1.9.2 (or earlier Ruby 1.9.3)
and Ruby 2.0?

Yes.

In Ruby 1.9.2, Socket.pack_sockaddr_un(name) raises an ArgumentError because name contains NUL, and extra NULs are not filled explicitly, but addrlen is always set to sizeof(struct sockaddr_un) by UNIXSocket.open, so the address of the socket is "\0foo" + "\0" * (UNIX_MATH_PATH - "\0foo".bytesize).
In Ruby 2.0, addr.bytesize < SIZE_OF_SOCKADDR_UN evaluates to true, so extra NULs are filled explicitly, and thus the address of the socket is "\0foo" + "\0" * (UNIX_MATH_PATH - "\0foo".bytesize).

"\0foo" + "\0" * (UNIX_MATH_PATH - "\0foo".bytesize) is used as the address of a socket in
the current Ruby 1.9.3 and earlier versions, and it's a valid address, so it should be able to be used as a socket address even if this bug is fixed.

[usa (Usaku NAKAMURA)]

• Status changed from Assigned to Closed
• % Done changed from 0 to 100

This issue was solved with changeset r39096.
Shugo, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

---

merge revision(s) 35474,35479,38939,38943,38963,38991,38994: [Backport #7786]

* ext/socket/raddrinfo.c (init_unix_addrinfo): support the longest
  path in sockaddr_un.
  (inspect_sockaddr): ditto.
  (addrinfo_mdump): ditto.
  (addrinfo_mload): ditto.
  (rsock_unixpath_str): new function.
  (rsock_unixpath): removed.
  (rsock_unixaddr): use rsock_unixpath_str.

* ext/socket/socket.c (sock_s_pack_sockaddr_un): support the longest
  path in sockaddr_un.
  (sock_s_unpack_sockaddr_un): ditto.
  (sock_s_gethostbyaddr): unused variable removed.

* ext/socket/unixsocket.c (rsock_init_unixsock): support the longest
  path in sockaddr_un.

* ext/socket/rubysocket.h (rsock_unixpath_str): declared.
  (rsock_unixpath): removed.

* test/socket/test_unix.rb: comment out test_nul because abstract unix
  sockets may contain NULs.

* ext/socket/socket.c (sock_s_pack_sockaddr_un): support the longest
  path in sockaddr_un, really.
  reported by nagachika.
  http://d.hatena.ne.jp/nagachika/20120426/ruby_trunk_changes_35474_35476

* ext/socket/raddrinfo.c (rsock_unixpath_len, init_unix_addrinfo),
  ext/socket/unixsocket.c (unixsock_connect_internal,
  rsock_init_unixsock): calculate the correct address length of
  an abstract socket.  Without this fix, sizeof(struct sockaddr_un)
  is specified as the length of an abstract socket for bind(2) or
  connect(2), so the address of the socket is filled with extra NUL
  characters.  See unix(7) for details.

* ext/socket/lib/socket.rb (unix_server_socket): don't access the
  file system if the platform is Linux and path starts with NUL,
  which means that the socket is an abstract socket.

* test/socket/test_unix.rb: related test.

* ext/socket/raddrinfo (rsock_unix_sockaddr_len): renamed from
  rsock_unixpath_len, because it returns not the length of the path,
  but the length of a socket address for the path.

* ext/socket/socket.c (sock_s_pack_sockaddr_un): calculate the
  correct address length of an abstract socket.

* test/socket/test_unix.rb: related test.

* ext/socket/unixsocket.c (rsock_init_unixsock): use rb_inspect()
  because rb_sys_fail_str() fails if its argument contains NUL.

* test/socket/test_unix.rb: related test.

* ext/socket/raddrinfo.c (rsock_unix_sockaddr_len): return
  sizeof(sa_familiy_t) if path is empty.  see "Autobind Feature" in
  unix(7) for details.

* ext/socket/lib/socket.rb (unix_socket_abstract_name?): treat an
  empty path as an abstract name.

* test/socket/test_unix.rb: related test.