Date: Thu, 8 Oct 1998 10:07:31 +0200 (SAT) From: Graham Wheeler <gram@cdsec.com> To: FreeBSD-gnats-submit@FreeBSD.ORG, freebsd-bugs@FreeBSD.ORG Cc: dag-erli@ifi.uio.no Subject: Re: bin/8183: Signal handlers in inetd.c are unsafe Message-ID: <199810080807.KAA07108@cdsec.com> In-Reply-To: <199810071000.DAA13971@freefall.freebsd.org> from "FreeBSD-gnats-submit@FreeBSD.ORG" at Oct 7, 98 03:00:01 am
next in thread | previous in thread | raw e-mail | index | archive | help
SEND-PR: -*- send-pr -*-
SEND-PR: Lines starting with `SEND-PR' will be removed automatically, as
SEND-PR: will all comments (text enclosed in `<' and `>').
SEND-PR:
SEND-PR: Please consult the send-pr man page `send-pr(1)' or the Texinfo
SEND-PR: manual if you are not sure how to fill out a problem report.
SEND-PR:
SEND-PR: Note that the Synopsis field is mandatory. The Subject (for
SEND-PR: the mail) will be made the same as Synopsis unless explicitly
SEND-PR: changed.
SEND-PR:
SEND-PR: Choose from the following categories:
SEND-PR:
SEND-PR: bin conf docs gnu i386 kern misc ports sparc
SEND-PR:
To: FreeBSD-gnats-submit@freebsd.org
Subject:
From: gram@cdsec.com
Reply-To: gram@cdsec.com
X-send-pr-version: 3.2
>Submitter-Id: current-users
>Originator: Graham Wheeler
>Organization: Citadel Data Security
>Confidential: no
>Synopsis: Signal handlers in inetd.c are unsafe
>Severity: serious
>Priority: medium
>Category: bin
>Release: FreeBSD 2.2.7
>Class: sw-bug
>Environment: Not important
>Description: The signal handlers in inetd.c make use of unsafe functions
> such as calls to allocate and free heap memory. This can
> result in heap corruption, which is typically manifested
> by inetd reporting the error message (from malloc or realloc):
>
> junk pointer: too low to make sense
>
>How-To-Repeat: The problem will usually occur if a signal is caught while
> in heap allocation function. The signal handler can also
> call these functions, which are not reentrant. As this is
> timing related, replicating it reliably can be difficult, but
> the problem is well known.
>
>Fix: The following patch fixes the problem, by modifying the signal
> handlers to write character flags to a pipe, and having the main
> select() loop read these flags and call the original handlers.
------------------ cut here ------------------------------------
--- /usr/src/usr.sbin/inetd.orig/inetd.c Tue Oct 6 16:04:43 1998
+++ /usr/src/usr.sbin/inetd/inetd.c Thu Oct 8 09:50:15 1998
@@ -172,6 +172,7 @@
struct servent *sp;
struct rpcent *rpc;
struct in_addr bind_address;
+int signalpipe[2];
struct servtab {
char *se_service; /* name of service */
@@ -217,7 +218,9 @@
void chargen_dg __P((int, struct servtab *));
void chargen_stream __P((int, struct servtab *));
void close_sep __P((struct servtab *));
-void config __P((int));
+void flag_signal __P((char));
+void flag_config __P((int));
+void config __P((void));
void daytime_dg __P((int, struct servtab *));
void daytime_stream __P((int, struct servtab *));
void discard_dg __P((int, struct servtab *));
@@ -234,10 +237,12 @@
char *nextline __P((FILE *));
void print_service __P((char *, struct servtab *));
void addchild __P((struct servtab *, int));
-void reapchild __P((int));
+void flag_reapchild __P((int));
+void reapchild __P((void));
void enable __P((struct servtab *));
void disable __P((struct servtab *));
-void retry __P((int));
+void flag_retry __P((int));
+void retry __P((void));
int setconfig __P((void));
void setup __P((struct servtab *));
char *sskip __P((char **));
@@ -408,12 +413,12 @@
sigaddset(&sa.sa_mask, SIGALRM);
sigaddset(&sa.sa_mask, SIGCHLD);
sigaddset(&sa.sa_mask, SIGHUP);
- sa.sa_handler = retry;
+ sa.sa_handler = flag_retry;
sigaction(SIGALRM, &sa, (struct sigaction *)0);
- config(SIGHUP);
- sa.sa_handler = config;
+ config();
+ sa.sa_handler = flag_config;
sigaction(SIGHUP, &sa, (struct sigaction *)0);
- sa.sa_handler = reapchild;
+ sa.sa_handler = flag_reapchild;
sigaction(SIGCHLD, &sa, (struct sigaction *)0);
sa.sa_handler = SIG_IGN;
sigaction(SIGPIPE, &sa, &sapipe);
@@ -428,6 +433,14 @@
(void)setenv("inetd_dummy", dummy, 1);
}
+ if (pipe(signalpipe) != 0)
+ {
+ syslog(LOG_ERR, "pipe: %%m");
+ exit(EX_OSERR);
+ }
+ FD_SET(signalpipe[0], &allsock);
+ if (signalpipe[0]>maxsock) maxsock = signalpipe[0];
+
for (;;) {
int n, ctrl;
fd_set readable;
@@ -447,6 +460,41 @@
}
continue;
}
+ /* handle any queued signal flags */
+ if (FD_ISSET(signalpipe[0], &readable))
+ {
+ int n;
+ if (ioctl(signalpipe[0], FIONREAD, &n) == 0)
+ {
+ while (--n >= 0)
+ {
+ char c;
+ if (read(signalpipe[0], &c, 1) == 1)
+ {
+ if (debug) warnx("Handling signal flag %c", c);
+ switch(c)
+ {
+ case 'A': /* sigalrm */
+ retry(); break;
+ case 'C': /* sigchld */
+ reapchild(); break;
+ case 'H': /* sighup */
+ config(); break;
+ }
+ }
+ else
+ {
+ syslog(LOG_ERR, "read: %m");
+ exit(EX_OSERR);
+ }
+ }
+ }
+ else
+ {
+ syslog(LOG_ERR, "ioctl: %m");
+ exit(EX_OSERR);
+ }
+ }
for (sep = servtab; n && sep; sep = sep->se_next)
if (sep->se_fd != -1 && FD_ISSET(sep->se_fd, &readable)) {
n--;
@@ -645,6 +693,20 @@
}
/*
+ * Add a signal flag to the signal flag queue for later handling
+ */
+
+void flag_signal(c)
+ char c;
+{
+ if (write(signalpipe[1], &c, 1) != 1)
+ {
+ syslog(LOG_ERR, "write: %m");
+ exit(EX_OSERR);
+ }
+}
+
+/*
* Record a new child pid for this service. If we've reached the
* limit on children, then stop accepting incoming requests.
*/
@@ -671,9 +733,15 @@
*/
void
-reapchild(signo)
+flag_reapchild(signo)
int signo;
{
+ flag_signal('C');
+}
+
+void
+reapchild()
+{
int k, status;
pid_t pid;
struct servtab *sep;
@@ -703,9 +771,14 @@
}
void
-config(signo)
+flag_config(signo)
int signo;
{
+ flag_signal('H');
+}
+
+void config()
+{
struct servtab *sep, *new, **sepp;
long omask;
@@ -882,8 +955,14 @@
}
void
-retry(signo)
+flag_retry(signo)
int signo;
+{
+ flag_signal('A');
+}
+
+void
+retry()
{
struct servtab *sep;
------------------ cut here ------------------------------------
--
Dr Graham Wheeler E-mail: gram@cdsec.com
Citadel Data Security Phone: +27(21)23-6065/6/7
Internet/Intranet Network Specialists Mobile: +27(83)253-9864
Firewalls/Virtual Private Networks Fax: +27(21)24-3656
Data Security Products WWW: http://www.cdsec.com/
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-bugs" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199810080807.KAA07108>