Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 17 Sep 2001 13:49:25 -0700 (PDT)
From:      Matt Dillon <dillon@earth.backplane.com>
To:        hackers@FreeBSD.org
Subject:   Proposed patch (was Re: bug in sshd - signal during free())
Message-ID:  <200109172049.f8HKnPj41790@earth.backplane.com>

next in thread | raw e-mail | index | archive | help
    I looked at the code and there is definitely a serious issue.  This
    proposed patch should solve the problem.  Here it is for review before
    I commit it and send a bug report off to the openssh folks.  I am testing
    it now.

						-Matt

Index: sshd.c
===================================================================
RCS file: /home/ncvs/src/crypto/openssh/sshd.c,v
retrieving revision 1.6.2.7
diff -u -r1.6.2.7 sshd.c
--- sshd.c	2001/03/04 15:13:08	1.6.2.7
+++ sshd.c	2001/09/17 20:45:54
@@ -134,6 +134,11 @@
 char *server_version_string = NULL;
 
 /*
+ * Indicates that a key-regeneration alarm occured.
+ */
+int received_regeneration;
+
+/*
  * Any really sensitive data in the application is contained in this
  * structure. The idea is that this structure could be locked into memory so
  * that the pages do not get written into swap.  However, there are some
@@ -260,19 +265,26 @@
 	fatal("Timeout before authentication for %s.", get_remote_ipaddr());
 }
 
-/*
- * Signal handler for the key regeneration alarm.  Note that this
- * alarm only occurs in the daemon waiting for connections, and it does not
- * do anything with the private key or random state before forking.
- * Thus there should be no concurrency control/asynchronous execution
- * problems.
- */
-/* XXX do we really want this work to be done in a signal handler ? -m */
 void
 key_regeneration_alarm(int sig)
 {
-	int save_errno = errno;
+	received_regeneration = 1;
+	/* Reschedule the alarm. */
+	signal(SIGALRM, key_regeneration_alarm);
+	alarm(options.key_regeneration_time);
+}
 
+/*
+ * Regenerate the keys.  Note that this alarm only occurs in the daemon
+ * waiting for connections, and it does not do anything with the
+ * private key or random state before forking.  However, it calls routines
+ * which may malloc() so we do not call this routine directly from the 
+ * signal handler.
+ */
+void
+key_regeneration(void)
+{
+	received_regeneration = 0;
 	/* Check if we should generate a new key. */
 	if (key_used) {
 		/* This should really be done in the background. */
@@ -292,10 +304,6 @@
 		key_used = 0;
 		log("RSA key generation complete.");
 	}
-	/* Reschedule the alarm. */
-	signal(SIGALRM, key_regeneration_alarm);
-	alarm(options.key_regeneration_time);
-	errno = save_errno;
 }
 
 void
@@ -854,6 +862,8 @@
 		for (;;) {
 			if (received_sighup)
 				sighup_restart();
+			if (received_regeneration)
+				key_regeneration();
 			if (fdset != NULL)
 				xfree(fdset);
 			fdsetsz = howmany(maxfd, NFDBITS) * sizeof(fd_mask);
@@ -994,6 +1004,8 @@
 	 */
 	alarm(0);
 	signal(SIGALRM, SIG_DFL);
+	received_regeneration = 0;
+
 	signal(SIGHUP, SIG_DFL);
 	signal(SIGTERM, SIG_DFL);
 	signal(SIGQUIT, SIG_DFL);

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-hackers" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200109172049.f8HKnPj41790>