SIGCHLD race *trivial* patch

[mouring at etoh.eviladmin.org]

It won't apply anywhere clean to the current CVS tree.. and when I shoe
horn it.. it provides me with no useful solution.

- Ben

On Thu, 25 Oct 2001, Nicolas Williams wrote:

> SSH_CHANNEL_INPUT_DRAINING and SSH_CHANNEL_OUTPUT_DRAINING channels are
> considered "still open" by channel_still_open(), so no, no data can be
> lost.
>
> That said, there's no harm in moving the check to the bottom of the
> loop.
>
> A new, cleaner patch is attached. Again, against 2.9p2 + stuff, but
> conceptually it should apply to any later releases that still have this
> bug.
>
> Cheers,
>
> Nico
>
> On Thu, Oct 25, 2001 at 04:30:09PM -0400, Nicolas Williams wrote:
> > I've done some testing and no, no data is lost. Look at the new
> > condition for breaking out of the server loop: if (there's no channels
> > AND there's no sessions).
> >
> > Of course, I'm not too familiar with how unsent buffers would be
> > handled. Can you explain?
> >
> > Perhaps the (no channels open && no sessions in use) check should be
> > made at the bottom of the loop, after channel_after_select(),
> > process_input() and process_output() have been called.
> >
> > Hmmm,
> >
> > Nico
> >
> > On Thu, Oct 25, 2001 at 03:25:12PM -0500, mouring at etoh.eviladmin.org wrote:
> > >
> > > I've not tested this, but have you verified that scp never loses any data?
> > > and the 'ssh site date'  NEVER loses data?
> > >
> > > - Ben
>
>
> Index: 2_9_p2_w_gss_krb5_named_keys.10/session.h
> --- 2_9_p2_w_gss_krb5_named_keys.10/session.h Tue, 26 Jun 2001 16:27:13 -0400 willian (OpenSSH/i/13_session.h 1.2 644)
> +++ 2_9_p2_w_gss_krb5_named_keys.12(w)/session.h Thu, 25 Oct 2001 14:58:32 -0400 willian (OpenSSH/i/13_session.h 1.3 644)
> @@ -28,6 +28,7 @@
>
>  void	do_authenticated(Authctxt *ac);
>
> +int	session_still_used();
>  int	session_open(int id);
>  void	session_input_channel_req(int id, void *arg);
>  void	session_close_by_pid(pid_t pid, int status);
> Index: 2_9_p2_w_gss_krb5_named_keys.10/session.c
> --- 2_9_p2_w_gss_krb5_named_keys.10/session.c Tue, 26 Jun 2001 16:27:13 -0400 willian (OpenSSH/i/14_session.c 1.3 644)
> +++ 2_9_p2_w_gss_krb5_named_keys.12(w)/session.c Thu, 25 Oct 2001 15:04:21 -0400 willian (OpenSSH/i/14_session.c 1.4 644)
> @@ -1533,18 +1533,18 @@
>  	exit(1);
>  }
>
> +static int did_init_sessions = 0;
>  Session *
>  session_new(void)
>  {
>  	int i;
> -	static int did_init = 0;
> -	if (!did_init) {
> +	if (!did_init_sessions) {
>  		debug("session_new: init");
>  		for(i = 0; i < MAX_SESSIONS; i++) {
>  			sessions[i].used = 0;
>  			sessions[i].self = i;
>  		}
> -		did_init = 1;
> +		did_init_sessions = 1;
>  	}
>  	for(i = 0; i < MAX_SESSIONS; i++) {
>  		Session *s = &sessions[i];
> @@ -1622,6 +1622,27 @@
>  	error("session_by_pid: unknown pid %d", pid);
>  	session_dump();
>  	return NULL;
> +}
> +
> +int
> +session_still_used()
> +{
> +	int i;
> +	if (!did_init_sessions) {
> +		debug("session_new: init");
> +		for(i = 0; i < MAX_SESSIONS; i++) {
> +			sessions[i].used = 0;
> +			sessions[i].self = i;
> +		}
> +		did_init_sessions = 1;
> +	}
> +	debug("session_still_used");
> +	for(i = 0; i < MAX_SESSIONS; i++) {
> +		Session *s = &sessions[i];
> +		if (s->used)
> +			return 1;
> +	}
> +	return 0;
>  }
>
>  int
> Index: 2_9_p2_w_gss_krb5_named_keys.10/serverloop.c
> --- 2_9_p2_w_gss_krb5_named_keys.10/serverloop.c Thu, 03 May 2001 16:12:13 -0400 jd (OpenSSH/i/16_serverloop 1.1 644)
> +++ 2_9_p2_w_gss_krb5_named_keys.12(w)/serverloop.c Thu, 25 Oct 2001 16:35:36 -0400 willian (OpenSSH/i/16_serverloop 1.3 644)
> @@ -259,6 +259,9 @@
>  		if (max_time_milliseconds == 0 || client_alive_scheduled)
>  			max_time_milliseconds = 100;
>
> +	if (!channel_still_open())
> +		max_time_milliseconds = 1000;
> +
>  	if (max_time_milliseconds == 0)
>  		tvp = NULL;
>  	else {
> @@ -599,6 +602,11 @@
>
>  		/* Process output to the client and to program stdin. */
>  		process_output(writeset);
> +
> +		/* Don't know if this is needed -- if it is, uncomment
> +		if (!channel_still_open() && child_terminated)
> +			break;
> +		*/
>  	}
>  	if (readset)
>  		xfree(readset);
> @@ -721,7 +729,8 @@
>  		if (child_terminated) {
>  			while ((pid = waitpid(-1, &status, WNOHANG)) > 0)
>  				session_close_by_pid(pid, status);
> -			child_terminated = 0;
> +			if (session_still_used())
> +				child_terminated = 0;
>  		}
>  		if (!rekeying)
>  			channel_after_select(readset, writeset);
> @@ -729,6 +738,8 @@
>  		if (connection_closed)
>  			break;
>  		process_output(writeset);
> +		if (child_terminated && !channel_still_open())
> +			break;
>  	}
>  	if (readset)
>  		xfree(readset);
>
> --
> -DISCLAIMER: an automatically appended disclaimer may follow. By posting-
> -to a public e-mail mailing list I hereby grant permission to distribute-
> -and copy this message.-
>
> Visit our website at http://www.ubswarburg.com
>
> This message contains confidential information and is intended only
> for the individual named.  If you are not the named addressee you
> should not disseminate, distribute or copy this e-mail.  Please
> notify the sender immediately by e-mail if you have received this
> e-mail by mistake and delete this e-mail from your system.
>
> E-mail transmission cannot be guaranteed to be secure or error-free
> as information could be intercepted, corrupted, lost, destroyed,
> arrive late or incomplete, or contain viruses.  The sender therefore
> does not accept liability for any errors or omissions in the contents
> of this message which arise as a result of e-mail transmission.  If
> verification is required please request a hard-copy version.  This
> message is provided for informational purposes and should not be
> construed as a solicitation or offer to buy or sell any securities or
> related financial instruments.
>
>