Project

General

Profile

Actions

Feature #1851

closed

Bus error in mod_compress

Added by thomasb about 16 years ago. Updated over 8 years ago.

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

Description

If a file is modified between stat_cache_get_entry (mod_compress.c+680) and the mmap of the file (mod_compress.c+489) lighty might die with a Bus error (if the new file is shorter that the
file in the stat-cache, otherwise it should only produce bad content)

To reproduce:
1) Make sure text/html-files are compressed
2) disable compress.cache-dir
3) Produce some load:

   $ ab -H "Accept-Encoding: deflate, gzip, x-gzip" -n10000 -c10 http://localhost/

4) Keep modifying the file:
   # cp /var/www/index.lighttpd.html .
   # while true; do cp index.lighttpd.html /var/www/index.lighttpd.html ; sleep .1; echo "short" > /var/www/index.lighttpd.html; sleep .1 ; done

Back-trace:

Core was generated by `/usr/sbin/lighttpd -f /etc/lighttpd/inicio-lighttpd.conf'.
Program terminated with signal 7, Bus error.
#0  0x00000032ae47a5a3 in memcpy () from /lib64/libc.so.6
(gdb) bt
#0  0x00000032ae47a5a3 in memcpy () from /lib64/libc.so.6
#1  0x00002baa3263b99d in deflateEnd () from /usr/lib64/libz.so.1
#2  0x00002baa3263c9ff in deflateParams () from /usr/lib64/libz.so.1
#3  0x00002baa3263bc70 in deflate () from /usr/lib64/libz.so.1
#4  0x00002baa3527e931 in deflate_file_to_buffer_gzip (srv=<value optimized out>, con=<value optimized out>, p=0x10c7c1d0, start=0x2baa35eba000 <Address 0x2baa35eba000 out of bounds>, st_size=16541, mtime=73)
    at mod_compress.c:245
#5  0x00002baa3527f1e3 in mod_compress_physical (srv=0x10bdc5a0, con=0x114571e0, p_d=0x10c7c1d0) at mod_compress.c:547
#6  0x00000000004181da in plugins_call_handle_subrequest_start ()
#7  0x0000000000408a0b in http_response_prepare ()
#8  0x000000000040ba7e in connection_state_machine ()
#9  0x0000000000407aa1 in main ()

Files

compress.diff (3.61 KB) compress.diff thomasb, 2008-12-18 14:11
Actions #1

Updated by thomasb about 16 years ago

The bug was identified in 1.4.18-1ubuntu1.4 and 1.4.20 (compiled for centos 64bits)

Actions #2

Updated by thomasb about 16 years ago

It turns out that verifying the sce stat-buffer is not enough, it seems like the mmap using MAP_SHARED causes problems, if the file is modified on disk during the compression.

Re-fstating the file and mapping using MAP_PRIVATE seems to work on my platform.

Actions #3

Updated by angie19 over 14 years ago

subscribing..

I have this bug reproducible on 2 servers, one CentOS 5.3 and the other CentOS 5.4 (both x86_64). One of the servers is our production server and this bug is causing lighttpd to crash around 10 times per day. I am using lighttpd angel to automatically restart it.

I am using the following mods:
"mod_access",
"mod_alias",
"mod_accesslog",
"mod_expire",
"mod_compress",
"mod_flv_streaming",
"mod_secdownload",
"mod_h264_streaming"

I applied the patch, but got a signal 9 when trying to start lighttpd then. Any ideas ?! Your help is really appreciated :)

Actions #4

Updated by icy over 14 years ago

  • Missing in 1.5.x set to No

Modifying files while having them mmap'd is always dangerous. There is a race condition between getting the size via (f)stat() and reading the mapped region in RAM. If the file shrinks, lighty might try to read a memory address which is not valid anymore... I don't see any easy way to reliably fix this.
One could think about remembering each mmap'd range and catch the bus error signal, check if it's in a known range and then throw an error for that connection.
Or just don't modify files that are already opened: write a new file and move it to the old file path in order to do an atomic overwrite.
That way mmap'd areas will still be valid as long as they are open.

Actions #5

Updated by angie19 over 14 years ago

The problem we had on production server was due to a constant change in .js files. This change was caused by Drupal's contrib module: "javascript_aggregator". The if statement checking for the JS files always returned false and thus the js files were always re-created.

Accordingly lighty was always trying to access a js file while being modified. Bug fix in drupal's module below: ==================================================================================================
@ -99,8 +99,7 @
$jsmin_file_path = $jsmin_file_name; ==================================================================================================

// Create the JSMinified file if it doesn't exist yet.
- if (!file_exists($jsmin_file_path)) {
-
+ if (!file_exists(file_create_path($jsmin_file_path))) {
if (variable_get('javascript_aggregator_jsminplus', FALSE)) {
// JSMin+ the contents of the aggregated file.
require_once(drupal_get_path('module', 'javascript_aggregator') .'/jsminplus.php');

I hope no one else runs into this problem. Thanks goes to my friend & senior sysadmin for fixing the bug.

Actions #6

Updated by gstrauss almost 9 years ago

mod_compress in current lighttpd 1.4.x (1.4.39) does not use mmap unless lighttpd was compiled with ./configure --enable-mmap, which is not the default. If your files are potentially being modified while being read, then configuring lighttpd with --enable-mmap is discouraged.

Probably ok to close this ticket. The prior "last update" to this ticket was in 2010, documenting the bugfix in the third-party application module (drupal)

Actions #7

Updated by stbuehler almost 9 years ago

  • Target version set to 1.4.x

I'd still like to handle SIGBUS or at least provide a runtime option.

Actions #8

Updated by gstrauss almost 9 years ago

That's fine. Please lower priority from "Normal" to "Low" and/or change from "Bug" to "Feature" so that it is easier to differentiate from other issues (such as those without easy workarounds). Thanks.

Actions #9

Updated by gstrauss over 8 years ago

  • Tracker changed from Bug to Feature

feature: catch and handle sigbus if mmap'd file changes during read.

workaround: do not use mmap if admin or untrusted users are modifying files "live", in-place.

It is much safer to copy to a temporary file, modify, and then atomically rename into place.

Actions #10

Updated by gstrauss over 8 years ago

  • Category set to mod_compress
  • Status changed from New to Patch Pending
  • Target version changed from 1.4.x to 1.4.40
Actions #11

Updated by gstrauss over 8 years ago

  • Status changed from Patch Pending to Fixed
Actions

Also available in: Atom