[Bug 3971] Race condition when using ControlMaster=auto with simultaneous connections

[bugzilla-daemon at mindrot.org]

https://bugzilla.mindrot.org/show_bug.cgi?id=3971

--- Comment #17 from ilya.konstantinov at gmail.com <ilya.konstantinov at gmail.com> ---
After applying the patch:
```
ssh -v -o ControlPath=/tmp/doesnotexist -o ControlMaster=no
git at github.com
...
debug1: trying to setup multiplex master socket at /tmp/doesnotexist
debug1: We will be multiplex master, not client.
...
debug1: control_persist_detach: backgrounding master process
debug1: forking to background
debug1: Entering interactive session.
...
debug1: mux_client_hello_exchange: read packet failed
muxclient: master hello exchange failed
Failed to connect to new control master
Connection to github.com closed by remote host.
client_loop: send disconnect: Broken pipe
```

Unrelated, did you intend for the original race to remain in the
ECONNREFUSED case? With the patch, two clients stumble on stale socket,
both unlink it but do not immediately replace with their own socket but
rather proceed to try and become masters. I'd rather we addressed both
races.

Since naive unlinking is a TOCTOU, consider something like (if you're
still avoiding locks...):

  replace_path = control_path + "-stale-socket"
  if link(temp_path, replace_path) == EEXIST { /* client */ }
  if connect(control_path) != ECONNREFUSED { /* client */ }
  if rename(replace_path, control_path) != 0 { /* client */ }

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.