| Summary: | race condition with ControlMaster=auto | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | Portable OpenSSH | Reporter: | Tony Finch <dot> | ||||||||
| Component: | ssh | Assignee: | Assigned to nobody <unassigned-bugs> | ||||||||
| Status: | CLOSED FIXED | ||||||||||
| Severity: | normal | CC: | djm, dot, dwmw2, jeff | ||||||||
| Priority: | P2 | ||||||||||
| Version: | 5.0p1 | ||||||||||
| Hardware: | All | ||||||||||
| OS: | All | ||||||||||
| Bug Depends on: | 1329 | ||||||||||
| Bug Blocks: | 1452 | ||||||||||
| Attachments: |
|
||||||||||
You don't seem to use the new 'test' argument to ssh_control_listener() You also new use it before it's declared, leading to the following error: ssh.c: In function ‘ssh_session’: ssh.c:997: warning: implicit declaration of function ‘ssh_control_listener’ ssh.c: At top level: ssh.c:1072: error: static declaration of ‘ssh_control_listener’ follows non-static declaration ssh.c:997: error: previous implicit declaration of ‘ssh_control_listener’ was here Created attachment 1461 [details]
updated patch
I verified that in combination with my patch for bug #1329, this still copes correctly with stale sockets. Thanks for looking at this patch. My original patch doesn't call ssh_control_listener() from ssh_session() so I think you introduced that error. I believe the control socket doesn't work with protocol version 1 because the latter doesn't support arbitrary multiplexed streams. If so it's wrong to add the call and wrong to re-arrange the code to make the call compile. Thanks for spotting that the test argument is redundant. Created attachment 1462 [details] fixed patch (In reply to comment #4) > My original patch doesn't call ssh_control_listener() from > ssh_session() so I think you introduced that error. So I did; sorry. I was applying this patch after the patch in bug #1330 to provide 'ControlPersist' support, and had to fix up a reject by hand -- incompetently, it would seem. This one puts it back where it should be. But also applies _after_ Alan Barrett's patch in bug #1330, which is hopefully also ready to be applied now. Fallback and recovery from mux socket errors is implemented in the latest patch to bug #1329. the patch in attachment #1309 [details] has been applied applied; will be in openssh-5.1
Mass update RESOLVED->CLOSED after release of openssh-5.1 |
Created attachment 1335 [details] Fix race in ControlMaster=auto There is a race in the setup of the ControlMaster socket in auto mode, as illustrated by the following command line: ssh -oControlMaster=auto -oControlPath=sock localhost 'sleep 1; echo 1' & ssh -oControlMaster=auto -oControlPath=sock localhost 'sleep 2; echo 2' & Both of the commands will try to start up as a control client, find that sock does not exist, and switch into control master mode. One will succeed in creating the control master socket and the other will fail and bomb. The attached patch eliminates this race by trying to create a control master socket first, and falling back to control client mode if master mode fails.