From 0f30be9a59662820c1cc5b3c6ab8512d13bc04cb Mon Sep 17 00:00:00 2001 From: djm Date: Fri, 6 Jan 2023 02:38:23 +0000 Subject: [PATCH] replace manual poll/ppoll timeout math with ptimeout API feedback markus / ok markus dtucker --- usr.bin/ssh/channels.c | 27 ++++++---------- usr.bin/ssh/channels.h | 5 +-- usr.bin/ssh/clientloop.c | 38 +++++++--------------- usr.bin/ssh/serverloop.c | 69 ++++++++++++++-------------------------- 4 files changed, 47 insertions(+), 92 deletions(-) diff --git a/usr.bin/ssh/channels.c b/usr.bin/ssh/channels.c index 8b9199599c4..a1a7f518934 100644 --- a/usr.bin/ssh/channels.c +++ b/usr.bin/ssh/channels.c @@ -1,4 +1,4 @@ -/* $OpenBSD: channels.c,v 1.421 2022/11/18 19:47:40 mbuhl Exp $ */ +/* $OpenBSD: channels.c,v 1.422 2023/01/06 02:38:23 djm Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -2368,7 +2368,7 @@ channel_garbage_collect(struct ssh *ssh, Channel *c) enum channel_table { CHAN_PRE, CHAN_POST }; static void -channel_handler(struct ssh *ssh, int table, time_t *unpause_secs) +channel_handler(struct ssh *ssh, int table, struct timespec *timeout) { struct ssh_channels *sc = ssh->chanctxt; chan_fn **ftab = table == CHAN_PRE ? sc->channel_pre : sc->channel_post; @@ -2377,8 +2377,6 @@ channel_handler(struct ssh *ssh, int table, time_t *unpause_secs) time_t now; now = monotime(); - if (unpause_secs != NULL) - *unpause_secs = 0; for (i = 0, oalloc = sc->channels_alloc; i < oalloc; i++) { c = sc->channels[i]; if (c == NULL) @@ -2398,24 +2396,17 @@ channel_handler(struct ssh *ssh, int table, time_t *unpause_secs) */ if (c->notbefore <= now) (*ftab[c->type])(ssh, c); - else if (unpause_secs != NULL) { + else if (timeout != NULL) { /* - * Collect the time that the earliest - * channel comes off pause. + * Arrange for poll wakeup when channel pause + * timer expires. */ - debug3_f("chan %d: skip for %d more " - "seconds", c->self, - (int)(c->notbefore - now)); - if (*unpause_secs == 0 || - (c->notbefore - now) < *unpause_secs) - *unpause_secs = c->notbefore - now; + ptimeout_deadline_monotime(timeout, + c->notbefore); } } channel_garbage_collect(ssh, c); } - if (unpause_secs != NULL && *unpause_secs != 0) - debug3_f("first channel unpauses in %d seconds", - (int)*unpause_secs); } /* @@ -2561,7 +2552,7 @@ channel_prepare_pollfd(Channel *c, u_int *next_pollfd, /* * Allocate/prepare poll structure */ void channel_prepare_poll(struct ssh *ssh, struct pollfd **pfdp, u_int *npfd_allocp, - u_int *npfd_activep, u_int npfd_reserved, time_t *minwait_secs) + u_int *npfd_activep, u_int npfd_reserved, struct timespec *timeout) { struct ssh_channels *sc = ssh->chanctxt; u_int i, oalloc, p, npfd = npfd_reserved; @@ -2585,7 +2576,7 @@ channel_prepare_poll(struct ssh *ssh, struct pollfd **pfdp, u_int *npfd_allocp, *npfd_activep = npfd_reserved; oalloc = sc->channels_alloc; - channel_handler(ssh, CHAN_PRE, minwait_secs); + channel_handler(ssh, CHAN_PRE, timeout); if (oalloc != sc->channels_alloc) { /* shouldn't happen */ diff --git a/usr.bin/ssh/channels.h b/usr.bin/ssh/channels.h index 18961014307..c04f5a7f971 100644 --- a/usr.bin/ssh/channels.h +++ b/usr.bin/ssh/channels.h @@ -1,4 +1,4 @@ -/* $OpenBSD: channels.h,v 1.143 2022/05/05 00:56:58 djm Exp $ */ +/* $OpenBSD: channels.h,v 1.144 2023/01/06 02:38:23 djm Exp $ */ /* * Author: Tatu Ylonen @@ -312,9 +312,10 @@ int channel_input_status_confirm(int, u_int32_t, struct ssh *); /* file descriptor handling (read/write) */ struct pollfd; +struct timespec; void channel_prepare_poll(struct ssh *, struct pollfd **, - u_int *, u_int *, u_int, time_t *); + u_int *, u_int *, u_int, struct timespec *); void channel_after_poll(struct ssh *, struct pollfd *, u_int); void channel_output_poll(struct ssh *); diff --git a/usr.bin/ssh/clientloop.c b/usr.bin/ssh/clientloop.c index 5b1d5d894af..19a5d4b192d 100644 --- a/usr.bin/ssh/clientloop.c +++ b/usr.bin/ssh/clientloop.c @@ -1,4 +1,4 @@ -/* $OpenBSD: clientloop.c,v 1.385 2022/11/29 22:41:14 dtucker Exp $ */ +/* $OpenBSD: clientloop.c,v 1.386 2023/01/06 02:38:23 djm Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -509,16 +509,15 @@ client_wait_until_can_do_something(struct ssh *ssh, struct pollfd **pfdp, u_int *npfd_allocp, u_int *npfd_activep, int rekeying, int *conn_in_readyp, int *conn_out_readyp) { - int timeout_secs, pollwait; - time_t minwait_secs = 0, now = monotime(); + struct timespec timeout; int ret; u_int p; *conn_in_readyp = *conn_out_readyp = 0; /* Prepare channel poll. First two pollfd entries are reserved */ - channel_prepare_poll(ssh, pfdp, npfd_allocp, npfd_activep, 2, - &minwait_secs); + ptimeout_init(&timeout); + channel_prepare_poll(ssh, pfdp, npfd_allocp, npfd_activep, 2, &timeout); if (*npfd_activep < 2) fatal_f("bad npfd %u", *npfd_activep); /* shouldn't happen */ @@ -542,30 +541,17 @@ client_wait_until_can_do_something(struct ssh *ssh, struct pollfd **pfdp, * some polled descriptor can be read, written, or has some other * event pending, or a timeout expires. */ - - timeout_secs = INT_MAX; /* we use INT_MAX to mean no timeout */ + set_control_persist_exit_time(ssh); + if (control_persist_exit_time > 0) + ptimeout_deadline_monotime(&timeout, control_persist_exit_time); if (options.server_alive_interval > 0) - timeout_secs = MAXIMUM(server_alive_time - now, 0); - if (options.rekey_interval > 0 && !rekeying) - timeout_secs = MINIMUM(timeout_secs, + ptimeout_deadline_monotime(&timeout, server_alive_time); + if (options.rekey_interval > 0 && !rekeying) { + ptimeout_deadline_sec(&timeout, ssh_packet_get_rekey_timeout(ssh)); - set_control_persist_exit_time(ssh); - if (control_persist_exit_time > 0) { - timeout_secs = MINIMUM(timeout_secs, - control_persist_exit_time - now); - if (timeout_secs < 0) - timeout_secs = 0; - } - if (minwait_secs != 0) - timeout_secs = MINIMUM(timeout_secs, (int)minwait_secs); - if (timeout_secs == INT_MAX) - pollwait = -1; - else if (timeout_secs >= INT_MAX / 1000) - pollwait = INT_MAX; - else - pollwait = timeout_secs * 1000; + } - ret = poll(*pfdp, *npfd_activep, pollwait); + ret = poll(*pfdp, *npfd_activep, ptimeout_get_ms(&timeout)); if (ret == -1) { /* diff --git a/usr.bin/ssh/serverloop.c b/usr.bin/ssh/serverloop.c index 221043617d2..5347e3355bc 100644 --- a/usr.bin/ssh/serverloop.c +++ b/usr.bin/ssh/serverloop.c @@ -1,4 +1,4 @@ -/* $OpenBSD: serverloop.c,v 1.232 2022/04/20 04:19:11 djm Exp $ */ +/* $OpenBSD: serverloop.c,v 1.233 2023/01/06 02:38:23 djm Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -162,12 +162,11 @@ client_alive_check(struct ssh *ssh) static void wait_until_can_do_something(struct ssh *ssh, int connection_in, int connection_out, struct pollfd **pfdp, - u_int *npfd_allocp, u_int *npfd_activep, u_int64_t max_time_ms, - sigset_t *sigsetp, int *conn_in_readyp, int *conn_out_readyp) + u_int *npfd_allocp, u_int *npfd_activep, sigset_t *sigsetp, + int *conn_in_readyp, int *conn_out_readyp) { - struct timespec ts, *tsp; + struct timespec timeout; int ret; - time_t minwait_secs = 0; int client_alive_scheduled = 0; u_int p; /* time we last heard from the client OR sent a keepalive */ @@ -176,14 +175,14 @@ wait_until_can_do_something(struct ssh *ssh, *conn_in_readyp = *conn_out_readyp = 0; /* Prepare channel poll. First two pollfd entries are reserved */ - channel_prepare_poll(ssh, pfdp, npfd_allocp, npfd_activep, - 2, &minwait_secs); + ptimeout_init(&timeout); + channel_prepare_poll(ssh, pfdp, npfd_allocp, npfd_activep, 2, &timeout); if (*npfd_activep < 2) fatal_f("bad npfd %u", *npfd_activep); /* shouldn't happen */ - - /* XXX need proper deadline system for rekey/client alive */ - if (minwait_secs != 0) - max_time_ms = MINIMUM(max_time_ms, (u_int)minwait_secs * 1000); + if (options.rekey_interval > 0 && !ssh_packet_is_rekeying(ssh)) { + ptimeout_deadline_sec(&timeout, + ssh_packet_get_rekey_timeout(ssh)); + } /* * if using client_alive, set the max timeout accordingly, @@ -194,15 +193,11 @@ wait_until_can_do_something(struct ssh *ssh, * analysis more difficult, but we're not doing it yet. */ if (options.client_alive_interval) { - uint64_t keepalive_ms = - (uint64_t)options.client_alive_interval * 1000; - - if (max_time_ms == 0 || max_time_ms > keepalive_ms) { - max_time_ms = keepalive_ms; - client_alive_scheduled = 1; - } if (last_client_time == 0) last_client_time = monotime(); + ptimeout_deadline_sec(&timeout, options.client_alive_interval); + /* XXX ? deadline_monotime(last_client_time + alive_interval) */ + client_alive_scheduled = 1; } #if 0 @@ -220,19 +215,10 @@ wait_until_can_do_something(struct ssh *ssh, * from it, then read as much as is available and exit. */ if (child_terminated && ssh_packet_not_very_much_data_to_write(ssh)) - if (max_time_ms == 0 || client_alive_scheduled) - max_time_ms = 100; - - if (max_time_ms == 0) - tsp = NULL; - else { - ts.tv_sec = max_time_ms / 1000; - ts.tv_nsec = 1000000 * (max_time_ms % 1000); - tsp = &ts; - } + ptimeout_deadline_ms(&timeout, 100); /* Wait for something to happen, or the timeout to expire. */ - ret = ppoll(*pfdp, *npfd_activep, tsp, sigsetp); + ret = ppoll(*pfdp, *npfd_activep, ptimeout_get_tsp(&timeout), sigsetp); if (ret == -1) { for (p = 0; p < *npfd_activep; p++) @@ -245,19 +231,18 @@ wait_until_can_do_something(struct ssh *ssh, *conn_in_readyp = (*pfdp)[0].revents != 0; *conn_out_readyp = (*pfdp)[1].revents != 0; + /* ClientAliveInterval probing */ if (client_alive_scheduled) { time_t now = monotime(); - - /* - * If the ppoll timed out, or returned for some other reason - * but we haven't heard from the client in time, send keepalive. - */ - if (ret == 0 || (last_client_time != 0 && last_client_time + - options.client_alive_interval <= now)) { + if (ret == 0 && + now > last_client_time + options.client_alive_interval) { + /* ppoll timed out and we're due to probe */ client_alive_check(ssh); last_client_time = now; - } else if (*conn_in_readyp) + } else if (ret != 0 && *conn_in_readyp) { + /* Data from peer; reset probe timer. */ last_client_time = now; + } } } @@ -332,7 +317,6 @@ server_loop2(struct ssh *ssh, Authctxt *authctxt) u_int npfd_alloc = 0, npfd_active = 0; int r, conn_in_ready, conn_out_ready; u_int connection_in, connection_out; - u_int64_t rekey_timeout_ms = 0; sigset_t bsigset, osigset; debug("Entering interactive session for SSH2."); @@ -358,13 +342,6 @@ server_loop2(struct ssh *ssh, Authctxt *authctxt) if (!ssh_packet_is_rekeying(ssh) && ssh_packet_not_very_much_data_to_write(ssh)) channel_output_poll(ssh); - if (options.rekey_interval > 0 && - !ssh_packet_is_rekeying(ssh)) { - rekey_timeout_ms = ssh_packet_get_rekey_timeout(ssh) * - 1000; - } else { - rekey_timeout_ms = 0; - } /* * Block SIGCHLD while we check for dead children, then pass @@ -375,7 +352,7 @@ server_loop2(struct ssh *ssh, Authctxt *authctxt) error_f("bsigset sigprocmask: %s", strerror(errno)); collect_children(ssh); wait_until_can_do_something(ssh, connection_in, connection_out, - &pfd, &npfd_alloc, &npfd_active, rekey_timeout_ms, &osigset, + &pfd, &npfd_alloc, &npfd_active, &osigset, &conn_in_ready, &conn_out_ready); if (sigprocmask(SIG_UNBLOCK, &bsigset, &osigset) == -1) error_f("osigset sigprocmask: %s", strerror(errno)); -- 2.20.1