Project

General

Profile

Actions

Bug #2159

closed

src/mod_cgi.c leaks opened file descriptors

Added by rata about 14 years ago. Updated about 14 years ago.

Status:
Fixed
Priority:
Normal
Category:
mod_cgi
Target version:
-
ASK QUESTIONS IN Forums:

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

Actions #1

Updated by stbuehler about 14 years ago

  • Status changed from New to Fixed
  • % Done changed from 0 to 100

Applied in changeset r2706.

Actions #2

Updated by rata about 14 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

Actions

Also available in: Atom