[ntp:hackers] Segfaults in ntp-dev

M. Warner Losh imp at bsdimp.com
Sun Jun 3 13:56:04 PDT 2007


In message: <466325B3.9050109 at udel.edu>
            "David L. Mills" <mills at udel.edu> writes:
: Guys,
: 
: The ntp-dev version has two known segfaults, maybe more. The fudge ... 
: time1 command segfaults in line 310 of ntp_config.c, but I can't find 
: anything wrong by looking at variables and pointers. I have a suspicion 
: though, and I'd like to ask for expert opinion.
: 
: A number of routines in Sachin's code declare a structure and return a 
: pointer to it. Obviously, the compiler can't put this on the stack, as 
: would ordinarily be the case. I would assume the compiler recognizes 
: this case in context and puts the structure on the heap. This makes me 
: nervous.

Actually, the compiler does put such structures on the stack.  A
pointer is returned to it, and you go to heck and die when you later
dereference it.  Or maybe you don't.  It all depends on the stack
dancing going on.  It may have the appearance of working, but in fact
you are begging for problems.  The segfault symptoms that Dr. Mills
observes are a common side effect.

Eg:

struct timeval *
foo()
{
	struct timeval tv;

	...
	return &tv;
}

is bogus.  The lifetime of tv is over as soon as the last instruction
in foo has executed.

Adding static like so:

struct timeval *
foo()
{
	static struct timeval tv;

	...
	return &tv;
}

is only less bogus because it is safe for single threaded programs,
but unsafe for threaded ones.  The lifetime of tv in this case is
'forever' but if there are multiple callers of foo, they smash the
previous results.  This may or may not present a problem depending on
how often foo() is called, by how many threads and how often the copy
is made.  It is always a programming mistake.

: When I taught programming languages in a previous life, dumb compilers 
: insisted local declarations be on the stack, so Sachin's style was 
: forbidden. My preferred style is to declare a local pointer, malloc the 
: structure (on the heap), set the local pointer to the structure and 
: return the local pointer. Machines of the day returned values in the AC 
: and MQ 36-bit registers and had only three index registers.
: 
: My question is, am I a wuss or a wise man?

I think you are a wise man.

Warner


More information about the hackers mailing list