Project

General

Profile

Actions

Bug #2905

closed

oversized fcgi requests should fail gracefully

Added by slawomir.pryczek over 6 years ago. Updated about 6 years ago.

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

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

2018-09-05_1234.png (36.7 KB) 2018-09-05_1234.png slawomir.pryczek, 2018-09-05 10:43
lingering-W-test-case.txt (32 KB) lingering-W-test-case.txt slawomir.pryczek, 2018-09-05 10:58
Actions #1

Updated by slawomir.pryczek over 6 years ago

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...

Actions #2

Updated by slawomir.pryczek over 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...

Actions #3

Updated by slawomir.pryczek over 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

Actions #4

Updated by slawomir.pryczek over 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

Actions #5

Updated by slawomir.pryczek over 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

Actions #6

Updated by gstrauss over 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.

Actions #7

Updated by gstrauss over 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;
        }

Actions #8

Updated by gstrauss about 6 years ago

  • Status changed from Patch Pending to Fixed
  • % Done changed from 0 to 100
Actions

Also available in: Atom