Date: Thu, 13 Oct 2005 21:30:23 GMT From: Nate Eldredge <nge@cs.hmc.edu> To: freebsd-bugs@FreeBSD.org Subject: Re: bin/45478: /bin/sh coredump Message-ID: <200510132130.j9DLUNga071234@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR bin/45478; it has been noted by GNATS.
From: Nate Eldredge <nge@cs.hmc.edu>
To: bug-followup@FreeBSD.org, olli@secnetix.de
Cc: keramida@freebsd.org
Subject: Re: bin/45478: /bin/sh coredump
Date: Thu, 13 Oct 2005 14:24:07 -0700 (PDT)
I have reproduced this and have a fix. It does seem to have been
INTON/INTOFF. To find where the signal was being handled inside malloc,
in gdb you can do
handle SIGINT nostop pass
(answer yes)
break exraise
command
where
continue
end
run
which will print a stack trace every time it handles the signal. Then you
look for malloc inside the trace.
I found the following offenders:
#11 0x0805bba7 in redirect (redir=0x0, flags=1) at redir.c:111
#11 0x0805e1a3 in setvar (name=0x80dd76e "_", val=0x810d0ac "true",
flags=0) at var.c:246
However, a casual glance revealed several other un-wrapped calls. 90% of
these are made via ckmalloc/ckrealloc/ckfree. Thus I think the easiest
and safest thing to do is to call INTON/INTOFF inside these functions.
They are safe to use recursively and have trivial overhead (incrementing a
counter). Here is a patch which does this. It is against CURRENT but I
imagine it will apply to other versions as well. After this patch I
cannot make it crash anymore.
I found only one unsafe use of malloc/free directly: in the umask builtin.
umaskcmd() calls setmode() which calls malloc, and then calls free. My
patch puts INTON/INTOFF around this as well.
By the way, I think the CPU usage thing is a red herring. I assume the
numbers are from top, and its CPU field is an average. It definitely
decays slowly to 0 after a process has been running, so it can report
nonzero even for a totally idle process. sh gets commands from the user
with a blocking read, so it must be idle while waiting for input.
diff -ur /usr/src/bin/sh/memalloc.c ./src/bin/sh/memalloc.c
--- /usr/src/bin/sh/memalloc.c Tue Apr 6 13:06:51 2004
+++ ./src/bin/sh/memalloc.c Thu Oct 13 14:02:30 2005
@@ -57,8 +57,10 @@
{
pointer p;
+ INTOFF;
if ((p = malloc(nbytes)) == NULL)
error("Out of space");
+ INTON;
return p;
}
@@ -70,11 +72,20 @@
pointer
ckrealloc(pointer p, int nbytes)
{
+ INTOFF;
if ((p = realloc(p, nbytes)) == NULL)
error("Out of space");
+ INTON;
return p;
}
+void
+ckfree(pointer p)
+{
+ INTOFF;
+ free(p);
+ INTON;
+}
/*
* Make a copy of a string in safe storage.
diff -ur /usr/src/bin/sh/memalloc.h ./src/bin/sh/memalloc.h
--- /usr/src/bin/sh/memalloc.h Tue Apr 6 13:06:51 2004
+++ ./src/bin/sh/memalloc.h Thu Oct 13 14:01:27 2005
@@ -48,6 +48,7 @@
pointer ckmalloc(int);
pointer ckrealloc(pointer, int);
+void ckfree(pointer);
char *savestr(char *);
pointer stalloc(int);
void stunalloc(pointer);
@@ -72,5 +73,3 @@
#define STTOPC(p) p[-1]
#define STADJUST(amount, p) (p += (amount), sstrnleft -= (amount))
#define grabstackstr(p) stalloc(stackblocksize() - sstrnleft)
-
-#define ckfree(p) free((pointer)(p))
diff -ur /usr/src/bin/sh/miscbltin.c ./src/bin/sh/miscbltin.c
--- /usr/src/bin/sh/miscbltin.c Fri Sep 9 12:59:41 2005
+++ ./src/bin/sh/miscbltin.c Thu Oct 13 14:00:13 2005
@@ -274,12 +274,14 @@
umask(mask);
} else {
void *set;
+ INTOFF;
if ((set = setmode (ap)) == 0)
error("Illegal number: %s", ap);
mask = getmode (set, ~mask & 0777);
umask(~mask & 0777);
free(set);
+ INTON;
}
}
return 0;
--
Nate Eldredge
nge@cs.hmc.edu
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200510132130.j9DLUNga071234>