peterson@SW.MCC.COM (James Peterson) (11/09/88)
VERSION:
R3
CLIENT MACHINE and OPERATING SYSTEM:
Sun 3/260 running SunOS 3.5
DISPLAY:
Sun CG2
AREA:
server
SYNOPSIS:
Server code has race condition which allows core dump.
DESCRIPTION:
If the server is killed with a signal, it calls KillServerResources
which, among other things, executes the following code:
for (i = 0; i < screenInfo.numScreens; i++)
(*screenInfo.screen[i].CloseScreen)(i, &screenInfo.screen[i]);
The screenInfo structure is initialized from NULL. In addition,
the screenInfo structure is re-initialized whenever all clients
disappear. Thus, in a running server, there are intervals when
screenInfo.numScreens is set to zero, and the server auto-configs
itself again. During this short time, it is possible for numScreens
to be incremented BEFORE CloseScreen is defined for that last screen.
For example, in the ddx/sun code in sunCG2MProbe, we have the following
code:
i = AddScreen(sunCG2MInit, argc, argv);
pScreenInfo->screen[index].CloseScreen = sunCG2MCloseScreen;
AddScreen (in dix/main.c) increments screenInfo.numScreens. For a short
time, CloseScreen is undefined. If the server is killed at this time,
KillServerResources is called, and attempts to call CloseScreen, but finds
a NULL pointer which causes a core dump.
REPEAT BY:
Hitting ^C at j-u-s-t the right time. I hit it once.
SAMPLE FIX:
Always check a pointer before using it:
for (i = 0; i < screenInfo.numScreens; i++)
if ((*screenInfo.screen[i].CloseScreen) != NULL)
(*screenInfo.screen[i].CloseScreen)(i, &screenInfo.screen[i]);