Bug #2159
closedsrc/mod_cgi.c leaks opened file descriptors
Description
When reporting http://redmine.lighttpd.net/issues/2158 I noticed that in file src/mod_cgi.c the function cgi_create_env leaks opened file descriptors in other cases (no just when fork() fails).
For example, if the first call to pipe() works ok, but the second one fails, the fds created in the first call to pipe() are leaked. Just closing the fds before the return statement would work, but perhaps its more convenient to add a label and a goto, because that would avoid the duplicated code on the error paths.
I'm not sure if there are more cases when they are leaked (I think I found more but I don't remember now). If I have time I will check again and report back, but anyways I wanted to report this since most probably I will forget to check again :)
Thanks a lot,
Rodrigo
Updated by stbuehler almost 15 years ago
- Status changed from New to Fixed
- % Done changed from 0 to 100
Applied in changeset r2706.
Updated by rata almost 15 years ago
Sorry for the delay, I've re checked and I think no other fd leak is left on
that function now.
The first two returns of the function are without anything allocated. The third
one is with the pipe call failure which is already fixed.
Then we have the switch where we fork. In case of error, the fds are closed. If
the default case is executed, we close at the beginning from_cgi_fds [1] and
to_cgi_fds [0]. So we still need to close from_cgi_fds [0] and to_cgi_fds [1].
Suppose the first if (if (con->request.content_length) is false. Then just after
that if, we close to_cgi_fds [1]. So the only one missing is from_cgi_fds [0] but
its is used with "fdevent_register" and lets assume it closes it (I've checked
it quicky and it seems ok) and also if we fail to use it with "fdevent_register"
it (the fd) is closed.
Also if the "if" statement (if (con->request.content_length)) is true, then all
uses inside that if of "from_cgi_fds [0]" are a close() to that fd before a
return statement, which is ok. And all uses of "to_cgi_fds [1]" are the same of
"from_cgi_fds [0]" except that it also write() to it. So it should be all ok if
I'm not missing something.
Also, I just noted that the default case before is the normal/regular path, so
its expected that there is no leak there (it would be noticied before :). So
when fork returns 0 (also the normal/regular path) all calls to close() are done
inconditionally and they seem correct (and I have not detected and fd leaks when
running normally). So I think that fds are not leaked in this function =)
Thanks a lot,
Rodrigo
Also available in: Atom