From owner-freebsd-hackers Mon Sep 17 13:49:35 2001 Delivered-To: freebsd-hackers@freebsd.org Received: from earth.backplane.com (earth-nat-cw.backplane.com [208.161.114.67]) by hub.freebsd.org (Postfix) with ESMTP id 80E0D37B40E for ; Mon, 17 Sep 2001 13:49:25 -0700 (PDT) Received: (from dillon@localhost) by earth.backplane.com (8.11.6/8.11.2) id f8HKnPj41790; Mon, 17 Sep 2001 13:49:25 -0700 (PDT) (envelope-from dillon) Date: Mon, 17 Sep 2001 13:49:25 -0700 (PDT) From: Matt Dillon Message-Id: <200109172049.f8HKnPj41790@earth.backplane.com> To: hackers@FreeBSD.org Subject: Proposed patch (was Re: bug in sshd - signal during free()) Sender: owner-freebsd-hackers@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG 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