|Summary:||large files / filesystem corruption can cause apache2 to eat up all available memory|
|Product:||Apache httpd-2||Reporter:||Tyler "Crackerjack" MacDonald <tyler>|
|Component:||Core||Assignee:||Apache HTTPD Bugs Mailing List <bugs>|
|Attachments:||Add a "LimitResponseFileSize" directive for default_handler to use.|
Description Tyler "Crackerjack" MacDonald 2006-06-13 22:22:12 UTC
I recently had some filesystem corruption that resulted in the file size of a .GIF to be reported as 65,536 terabytes, even though it's only 782 bytes: -rw-r--r-- 1 kirin kirin 656 Mar 21 2005 btn_profile_on.gif -rw-r--r-- 1 kirin kirin 866 Mar 21 2005 btn_register.gif -rw-r--r-- 1 kirin kirin 772 Mar 21 2005 btn_register_on.gif -rw-r--r-- 1 kirin kirin 72057594037928718 Mar 21 2005 btn_search.gif -rw-r--r-- 1 kirin kirin 709 Mar 21 2005 btn_search_on.gif -rw-r--r-- 1 kirin kirin 1045 Mar 21 2005 btn_users.gif -rw-r--r-- 1 kirin kirin 915 Mar 21 2005 btn_users_on.gif # cp btn_search.gif btn_search2.gif # ls -l btn_search2.gif -rw-r--r-- 1 root root 782 Jun 13 15:19 btn_search2.gif When apache tries to serve the corrupt file, it enters into a loop in default_handler that causes it to use up all available memory. ) bt #0 apr_bucket_alloc (size=52, list=0x8210390) at buckets/apr_buckets_alloc.c:136 #1 0xa7f8c790 in apr_bucket_simple_copy (a=0x8210670, b=0xaf9e12f0) at buckets/apr_buckets_simple.c:22 #2 0xa7f8a36c in apr_bucket_shared_copy (a=0x8210670, b=0xaf9e12f0) at buckets/apr_buckets_refcount.c:38 #3 0x0806d811 in default_handler (r=0x8320320) at core.c:3678 #4 0x080740f7 in ap_run_handler (r=0x8320320) at config.c:157 #5 0x080771e1 in ap_invoke_handler (r=0x8320320) at config.c:371 #6 0x08081c48 in ap_process_request (r=0x8320320) at http_request.c:258 #7 0x0807eeee in ap_process_http_connection (c=0x820c550) at http_core.c:172 #8 0x0807ae77 in ap_run_process_connection (c=0x820c550) at connection.c:43 #9 0x08085c24 in child_main (child_num_arg=<value optimized out>) at prefork.c:640 #10 0x08085f1a in make_child (s=<value optimized out>, slot=0) at prefork.c:736 #11 0x08085fda in startup_children (number_to_start=5) at prefork.c:754 #12 0x08086a44 in ap_mpm_run (_pconf=0x80a70a8, plog=0x80d5160, s=0x80a8f48) at prefork.c:975 #13 0x08061dcf in main (argc=134893856, argv=0x8153390) at main.c:717 I'm not sure if httpd would go into such a tailspin if the file *was* actually that huge or not, but it would be nice to see it be more resilient to large files and filesystem corruption.
Comment 1 Paul Querna 2006-06-14 05:46:30 UTC
The out of memory is from trying to split the huge file into buckets of AP_MAX_SENDFILE size to sendfile each section of the file. I don't believe there is anything we can do in this case, except to call the OOM abort function in APR, which has already been done in trunk. If you had a 64bit machine/OS/httpd (or where sizeof(apr_off_t) <= sizeof(apr_size_t)), I believe it would work since it would attempt to sendfile() it all in one bucket, rather than millions AP_MAX_SENDFILE size buckets.
Comment 2 Joe Orton 2006-06-14 09:19:13 UTC
I don't think it's unreasonable to expect that the server should either handle this situation correctly or at least fail gracefully without attempting to eat all your RAM. It would be possible to handle this quite correctly by storing an "insanely large file" in a bucket with unknown length. This could be done either as a modification to the file bucket or, I suppose, as a new bucket type. httpd could handle such a bucket without problems; apr_brigade_insert_file could do whatever is necessary to insert it. But I'm not entirely convinced it would be worth all the effort to fix this properly unless someone really wants to serve insanely large files with 32-bit httpd binaries. By my calculations apr_brigade_file_insert() will be using 36Mb of RAM in bucket structures per petabyte of file.
Comment 3 Nick Kew 2006-06-14 10:05:09 UTC
The server has effectively DOSed itself. A 500 response would be a sensible alternative. How to detect when to bail out? Maybe a LimitResponseSize directive (with a default value of AP_MAX_SENDFILE) would make sense as a sanity check?
Comment 4 Tyler "Crackerjack" MacDonald 2006-06-14 16:41:42 UTC
Created attachment 18464 [details] Add a "LimitResponseFileSize" directive for default_handler to use. This patch implements Nick's suggestion to have a config directive that limits the maximum size of a file we will send. I named the directive "LimitResponseFileSize" instead of "LimitResponseSize", because the directive only affects default_handler; other handlers could still send more information than this in a response.
Comment 5 William A. Rowe Jr. 2006-06-14 17:29:44 UTC
Might not be obvious, we have a keyword rather than changing subject ;-)
Comment 6 Tyler "Crackerjack" MacDonald 2006-06-14 17:30:59 UTC
ahh, didn't catch that. thanks. :-)
Comment 7 Joe Orton 2006-06-15 10:43:03 UTC
Please let's not add yet-another-config-directive just to paper over a bug. 1) this doesn't catch all the places where such files are added to brigades 2) there is more code added there than would be necessary to actually correctly handle such files 3) enforcing a 16Mb default response size is... unwise ;)
Comment 8 Tyler "Crackerjack" MacDonald 2006-06-16 16:23:32 UTC
I didn't think it would be that easy either... nothing's that easy. :) You're suggesting a "bucket with unknown length"... I'm not really sure what this means or how it'd work but it *sounds* interesting... > 1) this doesn't catch all the places where such files are added to brigades Fair enough > 2) there is more code added there than would be necessary to actually correctly handle such files Well that patch took me 10 minutes to whip up... so I guess that means you can supply a patch to fix this properly in 2 minutes? :) > 3) enforcing a 16Mb default response size is... unwise ;) People shouldn't have filed larger than that on their webservers! HTTP is not a file transfer protocol! ... kidding... I actually had no idea the limit was set that low. But that means that httpd is creating a bunch of these structures whenever it sends out any large file, not just an insanely large one... hmm... I'm going to stop thinking about this now.
Comment 9 Joe Orton 2006-06-19 14:02:41 UTC
Well, OK :) I *thought* this would be a 2 minute patch but modifying the FILE bucket to do this actually isn't possible, so my point (2) is out of the window. I still think that adding config directives for this is the wrong approach, however. It's actually annoyingly difficult to fix this in APR-util because apr_brigade_insert_file() cannot assume that the *whole* passed-in file must be inserted; so a new function with those semantics would be needed.