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

Exception when closing UNIXServer with abstract socket address #3936

Open
hangyas opened this issue Jan 24, 2017 · 7 comments · May be fixed by #4056
Open

Exception when closing UNIXServer with abstract socket address #3936

hangyas opened this issue Jan 24, 2017 · 7 comments · May be fixed by #4056

Comments

@hangyas
Copy link
Contributor

hangyas commented Jan 24, 2017

Abstract socket addresses start with a '\0', which causes a raise in UNIXSocket#close

require "socket"

UNIXSocket.open("\0foo") do |c|
  c.close
end
string contains null byte (ArgumentError)
0x42a9e7: *CallStack::unwind:Array(Pointer(Void)) at ??
0x42a97a: *CallStack#initialize:Array(Pointer(Void)) at ??
0x42a94a: *CallStack::new:CallStack at ??
0x42600e: *raise<ArgumentError>:NoReturn at ??
0x433b28: *String#check_no_null_byte:String at ??
0x44080a: *File::accessible?<String, Int32>:Bool at ??
0x4407f8: *File::exists?<String>:Bool at ??
0x43da8b: *UNIXServer#close:Nil at ??
0x425b75: ??? at ??
0x4288c9: main at ??
0x7fb6031b6291: __libc_start_main at ??
0x42546a: _start at ??
0x0: ??? at ??
@ysbaddaden
Copy link
Contributor

UNIXServer.new and UNIXServer.open should take an optional abstract parameter to inject the NULL byte.

@bew
Copy link
Contributor

bew commented Feb 19, 2017

UNIXSocket.new and UNIXSocket.open should also take the optional abstract parameter.

I'd like to do that as my first contribution, somewhere this week if you don't mind

@ysbaddaden
Copy link
Contributor

  • Both UNIXSocket and UNIXServer could take an abstract = false argument, and pass it down to UNIXAddress;
  • UNIXAddress could take an abstract = false argument, and only prepend the null byte to path when creating the LibC::SockaddrUn, so @path never contains a null byte;
  • We could provide an UNIXAddress#abstract? method, and maybe an UNIXSocket#abstract? method;
  • UNIXServer#close shouldn't search/delete the path if it's abstract, but still set @path to nil.

@bew
Copy link
Contributor

bew commented Feb 20, 2017

I agree with all your points, but I would add UNIXServer#abstract? also (if the user want the information back)

@ysbaddaden
Copy link
Contributor

ysbaddaden commented Feb 20, 2017

UNIXServer inherits from UNIXSocket so it will already have the abstract? method.

@bew
Copy link
Contributor

bew commented Feb 20, 2017

oh yes, I forgot that inheritance, thanks

@bew bew linked a pull request Feb 21, 2017 that will close this issue
@miketheman
Copy link
Contributor

#codetriage

This no longer errors out, currently shows this trace:

root@ubuntu-s-1vcpu-1gb-nyc3-01:~# uname -a
Linux ubuntu-s-1vcpu-1gb-nyc3-01 4.4.0-131-generic #157-Ubuntu SMP Thu Jul 12 15:51:36 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux

root@ubuntu-s-1vcpu-1gb-nyc3-01:~# crystal --version
Crystal 0.26.1 [391785249] (2018-08-27)

LLVM: 4.0.0
Default target: x86_64-unknown-linux-gnu


root@ubuntu-s-1vcpu-1gb-nyc3-01:~# crystal run foo.cr
Unhandled exception: connect: Connection refused (Errno)
  from /usr/share/crystal/src/socket/unix_socket.cr:0:7 in 'initialize'
  from /usr/share/crystal/src/socket/unix_socket.cr:18:3 in 'new'
  from /usr/share/crystal/src/socket/unix_socket.cr:40:5 in '__crystal_main'
  from /usr/share/crystal/src/crystal/main.cr:97:5 in 'main_user_code'
  from /usr/share/crystal/src/crystal/main.cr:86:7 in 'main'
  from /usr/share/crystal/src/crystal/main.cr:106:3 in 'main'
  from __libc_start_main
  from _start
  from ???

The work in #4056 hasn't been merged in yet, so it's unclear whether this is meant to work?

I've tried some strace to see what the client is doing when attempting to establish a connection.

## session 1 (server)
$ socat ABSTRACT-LISTEN:foo -

## session 2 (client)
$ netstat -l | grep foo
unix  2      [ ACC ]     STREAM     LISTENING     27903    @foo

$ crystal build client.cr && strace ./client
...
connect(10, {sa_family=AF_LOCAL, sun_path=@"foo"}, 110) = -1 ECONNREFUSED (Connection refused)

So it appears that something is being translated to the correct sun_path, but it's not able to make the connection? I'm not certain of the next steps here - but the original crash is gone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants