Backport #7786

fix for abstract namespace

Added by Shugo Maeda about 1 year ago. Updated about 1 year ago.

[ruby-core:51870]
Status:Closed
Priority:Normal
Assignee:Usaku NAKAMURA

Description

=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 sockaddrun).
However, on Linux, if sun
path starts with NUL, which means that the socket's address is in abstract namespace, the first (addrlen - sizeof(safamilyt)) 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 rsockunixsockaddr_len() to calculate the correct address length on Linux.

Besides that, the patch includes the following changes.

  • fix Socket.unixserversocket not to access the file system if the specified name is in abstract namespace.
  • fix rsockinitunixsock to use rbinspect() to avoid ArgumentError in rbsysfailstr() 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

abstract_namespace_fix.diff Magnifier (20.7 KB) Shugo Maeda, 02/05/2013 10:24 PM

Associated revisions

Revision 39096
Added by Usaku NAKAMURA about 1 year ago

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.

History

#1 Updated by Usaku NAKAMURA about 1 year ago

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"? :)

#2 Updated by Shugo Maeda about 1 year ago

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.

#3 Updated by Akira Tanaka about 1 year ago

2013/2/5 shugo (Shugo Maeda) redmine@ruby-lang.org:

  • 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

#4 Updated by Shugo Maeda about 1 year ago

akr (Akira Tanaka) wrote:

2013/2/5 shugo (Shugo Maeda) redmine@ruby-lang.org:

  • 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"

OFFSETOFSUNPATH = 2
UNIX
PATHMAX = 108
SIZE
OFSOCKADDRUN = OFFSETOFSUNPATH + UNIXPATHMAX
name = "\0foo"
begin
addr = Socket.pack
sockaddrun(name)
if addr.bytesize < SIZE
OFSOCKADDRUN
# append extra NULs for broken abstract namespace handling.
sunpath = name + "\0" * (UNIXPATHMAX - name.bytesize)
else
sun
path = name
end
rescue ArgumentError
sunpath = name
end
UNIXSocket.open(sun
path) do
# ...
end

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

#5 Updated by Akira Tanaka about 1 year ago

2013/2/6 shugo (Shugo Maeda) redmine@ruby-lang.org:

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"

OFFSETOFSUNPATH = 2
UNIX
PATHMAX = 108
SIZE
OFSOCKADDRUN = OFFSETOFSUNPATH + UNIXPATHMAX
name = "\0foo"
begin
addr = Socket.pack
sockaddrun(name)
if addr.bytesize < SIZE
OFSOCKADDRUN
# append extra NULs for broken abstract namespace handling.
sunpath = name + "\0" * (UNIXPATHMAX - name.bytesize)
else
sun
path = name
end
rescue ArgumentError
sunpath = 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

#6 Updated by Shugo Maeda about 1 year ago

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.packsockaddrun(name) raises an ArgumentError because name contains NUL, and extra NULs are not filled explicitly, but addrlen is always set to sizeof(struct sockaddrun) by UNIXSocket.open, so the address of the socket is "\0foo" + "\0" * (UNIXMATHPATH - "\0foo".bytesize).
In Ruby 2.0, addr.bytesize < SIZE
OFSOCKADDRUN evaluates to true, so extra NULs are filled explicitly, and thus the address of the socket is "\0foo" + "\0" * (UNIXMATHPATH - "\0foo".bytesize).

"\0foo" + "\0" * (UNIXMATHPATH - "\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.

#7 Updated by Usaku NAKAMURA about 1 year ago

  • 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.

Also available in: Atom PDF