Bug #2905
closedoversized fcgi requests should fail gracefully
Description
Hi Guys,
in my company, we have recently discovered that something very bad is happening with lighttpd 1.4.49 and 1.4.50 regarding fcgi handling. Basically from time to time our logs are bombarded with GIGABYTES of
2018-09-04 15:06:51: (mod_fastcgi.c.230) fcgi-request is already in use: 1 2018-09-04 15:06:51: (mod_fastcgi.c.230) fcgi-request is already in use: 1 2018-09-04 15:06:51: (mod_fastcgi.c.230) fcgi-request is already in use: 1 2018-09-04 15:06:51: (mod_fastcgi.c.230) fcgi-request is already in use: 1 2018-09-04 15:06:51: (mod_fastcgi.c.230) fcgi-request is already in use: 1
Basically we're doing thousands requests per second, and this situation happens not often, so it seems to be some race condition or maybe lack of locking, i think impossible to reproduce. Which can be tracked to
https://github.com/lighttpd/lighttpd1.4/blob/master/src/mod_fastcgi.c line 230
This was not happening in older versions, so probably some regression related to fcgi handling, not very familiar with this code, so maybe someone who made changes to fcgi during last half year up to a year may have some idea?
Thanks
Files
Updated by slawomir.pryczek about 6 years ago
- File 2018-09-05_1234.png 2018-09-05_1234.png added
Okay, i made a test case which i'll share soon because for now it has private data but maybe i'll be able to find this error on my own. So currently the thing is that after receiving large php request the thread "stops responding" in write (W) state... while the log gets populated with these errors.
Maybe someone has idea where i should look, otherwise i'll try to come up with test case without private data in it...
Updated by slawomir.pryczek about 6 years ago
Thought it'll be more hard to reproduce this but it appears that this bug is very simple to get, so here is a working test-case. We only need to call php file with large enough GET data, and it automatically gets a thread into some kind of broken state for minutes while logs get flooded with fcg-request already in use...
Updated by slawomir.pryczek about 6 years ago
I'll keep updating this ticket with new data as the issue seems serious, so from what i discovered after some code edits is that the path that returns HTTP-400 for bad request is broken in .49 and .50, while in .45 it's working fine.
The problem manifests that
con->http_status = 400; buffer_free(fcgi_env); return HANDLER_FINISHED;
Which can be found in mod_fastcgi.c no longer causes the server to return http-400, the server just goes into infinite loop
Updated by slawomir.pryczek about 6 years ago
In .45 - fcgi_create_env() will return HANDLER_ERROR, on error. In .49/.50 it'll return HANDLER_FINISHED on error, which is causing that loop because when changed - the error is gone. Now the question is why http_cgi_headers(..) is not able to correctly read _GET payload larger than 32kb
Updated by slawomir.pryczek about 6 years ago
Ok, so to fix the bug it's necessary to return HANDLER_ERROR, instead of HANDLER_FINISHED for error(s) in fcgi_create_env.
Problem with sending fcgi-data larger than 32k is that the function fcgi_env_add(...) relies on constant:
#define FCGI_MAX_LENGTH 0xffff
After increasing it the server can't process large messages anyway as fcgi header size is limited to 16bit... not sure why it seems to be limited to 32k instead of 64k, but maybe it might be possible to send multiple headers...
Anyway would be nice to add some warning too when server.max-request-size or server.max-request-field-size exceeds 31k maybe
Updated by gstrauss about 6 years ago
Thanks for the repro. I'll have more time to dig into it this weekend.
...
Now the question is why http_cgi_headers(..) is not able to correctly read _GET payload larger than 32kb
...
After increasing it the server can't process large messages anyway as fcgi header size is limited to 16bit... not sure why it seems to be limited to 32k instead of 64k, but maybe it might be possible to send multiple headers...
lighttpd provides QUERY_STRING and REQUEST_URI in the CGI environment, so 32k + 32k = 64k.
Updated by gstrauss about 6 years ago
- Subject changed from Something really critical is happening with fcgi-handling to oversized fcgi requests should fail gracefully
- Status changed from New to Patch Pending
- Target version changed from 1.4.x to 1.4.51
diff --git a/src/mod_fastcgi.c b/src/mod_fastcgi.c --- a/src/mod_fastcgi.c +++ b/src/mod_fastcgi.c @@ -243,6 +243,7 @@ static handler_t fcgi_create_env(server *srv, handler_ctx *hctx) { if (0 != http_cgi_headers(srv, con, &opts, fcgi_env_add, fcgi_env)) { con->http_status = 400; + con->mode = DIRECT; buffer_free(fcgi_env); return HANDLER_FINISHED; } else { diff --git a/src/mod_scgi.c b/src/mod_scgi.c --- a/src/mod_scgi.c +++ b/src/mod_scgi.c @@ -160,6 +160,7 @@ static handler_t scgi_create_env(server *srv, handler_ctx *hctx) { if (0 != http_cgi_headers(srv, con, &opts, scgi_env_add, scgi_env)) { buffer_free(scgi_env); con->http_status = 400; + con->mode = DIRECT; return HANDLER_FINISHED; }
Updated by gstrauss about 6 years ago
- Status changed from Patch Pending to Fixed
- % Done changed from 0 to 100
Applied in changeset 4992c4de10bf6bf8b38f2fc1c68e5fda8c697456.
Also available in: Atom