Mailing List Archive

> I can't see any side-effect after commenting out both
> free_env() calls after cgi_stub()
>
> The core dumping stops.

hmm, the original mail has vanished...

I was seeing

httpd: caught SIGBUS, dumping core

after a script successfully redirected to another script or to
a html which included cgi.

calls to free_env() after a call to cgi_stub() were the cause.

thoughts anyone ?

robh

>
> Yuk Yuk Yuk. The fault is in make_env(), which wasn't done any favours by
> E25_custom_responses.txt.
>

> which will make it even more likely that the global gets trashed.
>
> Solution:
> 1 Rob: remove your terrible hack to new_env, instead change the calls to
> new_env to updated in_headers_env where appropriate.

Are there cases when updating "in_headers_env" is inappropriate ?
I didn't think so, hence the reason for doing this in the routine
instead of outside as you propose.

robh

> > I can't see any side-effect after commenting out both
> > free_env() calls after cgi_stub()
> >
> > The core dumping stops.
>
> hmm, the original mail has vanished...
>
> I was seeing
>
> httpd: caught SIGBUS, dumping core
>
> after a script successfully redirected to another script or to
> a html which included cgi.
>
> calls to free_env() after a call to cgi_stub() were the cause.
>
> thoughts anyone ?
>
> robh

Yuk Yuk Yuk. The fault is in make_env(), which wasn't done any favours by
E25_custom_responses.txt.

It allocates a new array (newenv) larger than env, copies the pointers
across, and _frees the old env_. i.e. (simplified)

char **new_env(char **env, int to_add, int *pos)
{
int x;
char **newenv;

for(x=0; env[x]; x++);
newenv = (char **)malloc((to_add+x+1)*(sizeof(char *)));
for(x=0; env[x]; x++) newenv[x] = env[x];
*pos = x;
free(env);
return newenv;
}


Some parts of the CGI code and server-side includes code contain, essentially:
(see send_parsed_file and exec_cgi_script)
{
env = new_env(in_headers_env, ..)
[add more headers to env]
[use env]
free_env(env)
}

Net result; in_headers_env, the global created by get_mime_headers(), is
free'd, as are all the strings it contained. Better not try and use it again,
especially after calling malloc()...

I don't think E25 helped the situation; this changed new_env to do

char **new_env(char **env, int to_add, int *pos)
{
...
*pos = x;
free(env);
in_headers_env = newenv;
return newenv;
}

which will make it even more likely that the global gets trashed.

Solution:
1 Rob: remove your terrible hack to new_env, instead change the calls to
new_env to updated in_headers_env where appropriate.

2 Split new_env into two routines.
a) enlarge_env, for modifying an environment; this would free the
env pointer passed to it
b) dup_env, for duplicating an environment; this would strdup all the
environmnent strings, as well as the pointer array, and _not_ free
the original pointer array.

David.

RobH wrote:
> Did you respond to my question about whether there were
> any inappropriate times to set the global env to the
> new one ? i.e. is there any need to move that bit of
> code outside of the routine ?
>
> We need a fix for this for 0.6.3, so the sooner I know, the
> sooner I can look into fixing it, if indeed it does need
> more of a fix than my existing patch (3 free_env calls removed)

I've had a look, and cannot see any obvious problems with having
new_env always update the global env. Except, it is possible to
have nested redirects?

I agree we need to fix the problem for 0.6.3. Simply deleting
free_env is probably ok for now, we can live with the store leak.
In fact, if you delete the calls to free_env, then you have to
update the global env.

Though I really don't like the way the routines in util.c access
the same variable both as a global and as a parameter...

David.