Bug 53131 - apr_file_write() incorrectly used
Summary: apr_file_write() incorrectly used
Status: RESOLVED FIXED
Alias: None
Product: Apache httpd-2
Classification: Unclassified
Component: All (show other bugs)
Version: 2.5-HEAD
Hardware: PC Linux
: P2 major (vote)
Target Milestone: ---
Assignee: Apache HTTPD Bugs Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-04-23 04:46 UTC by Nicolas Viennot
Modified: 2012-04-26 21:55 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Nicolas Viennot 2012-04-23 04:46:25 UTC
In most cases, developers assume that when apr_file_write() returns APR_SUCCESS, everything went well, and move on.
In reality, apr_file_write() returns APR_SUCCESS if *some* bytes got written successfully. One of the many cases when this could happen is when the server receive a signal in the middle of the write() system call. The kernel will not return -EINTR, but the number of bytes that have been written so far. The caller needs to check that all the bytes to write have been successfully written. This is more or less implemented by apr_file_write_full().

I wrote a fix for mod_log_config because log file corruption is sad.
Patch is here: https://github.com/nviennot/apache-httpd/commit/fcbc7ed94
Note: flush_log() returns void, albeit apr_file_write_full() may fail.


This issue is present in different part of the code base. Following a list of highly suspicious calls:
https://github.com/apache/httpd/blob/62db628c/modules/slotmem/mod_slotmem_shm.c#L169
https://github.com/apache/httpd/blob/62db628c/modules/mappers/mod_rewrite.c#L1404
https://github.com/apache/httpd/blob/62db628c/modules/mappers/mod_rewrite.c#L1414
https://github.com/apache/httpd/blob/62db628c/modules/generators/mod_cgid.c#L1158
https://github.com/apache/httpd/blob/62db628c/modules/generators/mod_cgi.c#L298
https://github.com/apache/httpd/blob/62db628c/modules/cache/mod_cache_disk.c#L742
https://github.com/apache/httpd/blob/62db628c/modules/cache/mod_cache_disk.c#L752
https://github.com/apache/httpd/blob/62db628c/modules/cache/mod_cache_disk.c#L901
https://github.com/apache/httpd/blob/62db628c/modules/cache/mod_cache_disk.c#L910
https://github.com/apache/httpd/blob/62db628c/modules/cache/mod_cache_disk.c#L977
https://github.com/apache/httpd/blob/62db628c/modules/cache/mod_cache_disk.c#L988
https://github.com/apache/httpd/blob/62db628c/modules/cache/mod_cache_disk.c#L1054
Comment 1 Stefan Fritsch 2012-04-26 21:46:36 UTC
replaced apr_file_write{,v} with apr_file_write{,v}_full in r1331110
some error checking is still missing.
Comment 2 Nicolas Viennot 2012-04-26 21:55:38 UTC
LGTM