Bug 3071 - unhandled EINTR while connecting causes ssh to exit prematurely
Summary: unhandled EINTR while connecting causes ssh to exit prematurely
Status: CLOSED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: ssh (show other bugs)
Version: 8.0p1
Hardware: amd64 Linux
: P5 trivial
Assignee: Assigned to nobody
URL:
Keywords:
Depends on:
Blocks: V_8_4
  Show dependency tree / graph
 
Reported: 2019-09-17 03:40 AEST by Matthew Avery
Modified: 2020-10-02 14:55 AEST (History)
2 users (show)

See Also:

Attachments
misc.c path (620 bytes, patch)
2019-09-17 03:40 AEST, Matthew Avery 		no flags Details | Diff
handle EINTR in connect_timeout() and waitfd() (980 bytes, patch)
2020-05-29 15:16 AEST, Damien Miller 		dtucker: ok+
Details | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Matthew Avery 2019-09-17 03:40:41 AEST 
Created attachment 3329 [details]
misc.c path

We've observed some machines always returning one EINTR during a connection where a ConectTimeout is specified on the command line. ssh exits prematurely as the errno == EINTR is processed as hard error instead of being filtered out and retried in the select loop.

It looks to be correlated to migrating to Ubuntu 18.04.4 but the change is trivial enough I didn't dig much further into why it's happening.

patch (attached) is trivial.

web diff of the patch:

https://github.com/matthewaveryusa/openssh-portable/commit/127093f0a177bc4f5316453f777582f113fe18d9#diff-d93946eccdb07e73380c55caff5625f9

-Matt
Comment 1 Damien Miller 2020-05-29 15:16:49 AEST 
Created attachment 3401 [details]
handle EINTR in connect_timeout() and waitfd()

I think the timeout_connect() case in the previous patch is slightly wrong. The connect() call did not succeed and needs to be retried.

Darren, is this even necessary with your recent signal changes?
Comment 2 Darren Tucker 2020-05-29 15:37:53 AEST 
(In reply to Damien Miller from comment #1)
> Darren, is this even necessary with your recent signal changes?

I think it'll now work correctly anything that sets SA_RESTART but we don't do that on every platform for reasons, so this diff is probably needed for their benefit.
Comment 3 Darren Tucker 2020-05-29 15:41:03 AEST 
Comment on attachment 3401 [details]
handle EINTR in connect_timeout() and waitfd()

>+		} else if (errno == EINTR)
>+			continue;

maybe also EAGAIN?
Comment 4 Damien Miller 2020-06-26 15:18:03 AEST 
This has been committed and will be in OpenSSH 8.4, due in a few months - thanks
Comment 5 Darren Tucker 2020-10-02 14:55:06 AEST 
Mass close of all bugs fixed in 8.4 release.