mobile-shell / mosh

Mobile Shell
https://mosh.org
GNU General Public License v3.0
12.59k stars 729 forks source link

Better handling of fds 0/1/2 #1233

Open cgull opened 1 year ago

cgull commented 1 year ago

I ran into this while working on what is now #1232. mosh-server does not adequately handle the problem of standard file descriptors being closed. In my development, er, over two years ago, I ran into a situation where mosh-server had started with fd 2 closed, it opened /dev/urandom on fd 2, and then later reopened fd 2 on /dev/null. Then, when it tried to actually read fd 2 to get entropy, it got 0 bytes instead, and got caught in a never-ending loop involving an exception being caught and the action being retried. I don't remember for sure, but I think that left the Mosh session hung in mosh.

This work does 3 things:

I can't now reproduce the hang, unfortunately. Both Mosh and my OSes have changed since hten. I tried to write a test for this issue, but between not being able to reproduce the problem and mosh-server being difficult to keep track of because it double-forks, I've given up for now. Should any of you doubt there's a problem, strace -o strace.log -f mosh-server new 2>&- will convince you fairly quickly. And while I didn't experience any actual crypto failure, even the possibility of confused crypto code is a rather bad smell.

(Plus, we burned a couple of weeks chasing this problem down at $WORK once.)

achernya commented 1 year ago

@cgull would you mind rebasing this PR?