Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sanity check #5

Closed
wants to merge 7,707 commits into from
Closed

Sanity check #5

wants to merge 7,707 commits into from

Conversation

marcstern
Copy link

In case pfd.fd < 0, the poll() function waits until the time-out is expired.
This problem happens when, for instance, ModSecurity drops a connection in httpd. In this case, ressources gets blocked until the timeout expires.
In this situation, we should immediately return an error.
Same problem/solution in v1.6.x

ylavic and others added 30 commits July 18, 2014 00:25
git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1613086 13f79535-47bb-0310-9956-ffa450edef68
PR: 56627
Submitted by: Fredrik Fornwall <fredrik fornwall.net>, trawick


git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1618843 13f79535-47bb-0310-9956-ffa450edef68
on error path.

Submitted by: Philip Martin <philip.martin wandisco.com>
Reviewed by: trawick


git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1619438 13f79535-47bb-0310-9956-ffa450edef68
instead of linker def file due to bug with mwldnlm linker
where patch version > 26 is ignored from def file.


git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1621593 13f79535-47bb-0310-9956-ffa450edef68
and points out that they aren't useful anyway:

  http://public.kitware.com/Bug/view.php?id=14600

remove .pdb handling for static libs regardless of cmake version


git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1625175 13f79535-47bb-0310-9956-ffa450edef68
driver support for apr_dbd_transaction_end().

PR: 56330
Submitted by: Weiqiang Li <weiqiang_li hotmail.com>


git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1625247 13f79535-47bb-0310-9956-ffa450edef68
…versions

of NSS detect key sizes correctly, leading to test failures.


git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1626561 13f79535-47bb-0310-9956-ffa450edef68
git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1626658 13f79535-47bb-0310-9956-ffa450edef68
     returning IPv4 addresses if any IPv6 addresses were returned. 
     [Eric Covener]




git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1634615 13f79535-47bb-0310-9956-ffa450edef68
…rect

  return value in case we actually escape the string.

PR: 57230
Submitted by: <aduryagin gmail.com>
Reviewed by: rpluem



git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1642159 13f79535-47bb-0310-9956-ffa450edef68
Submitted By: Pat Odonnell <patod us ibm com>
Committed By: covener



git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1646891 13f79535-47bb-0310-9956-ffa450edef68
git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1646892 13f79535-47bb-0310-9956-ffa450edef68
git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1646893 13f79535-47bb-0310-9956-ffa450edef68
git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1648830 13f79535-47bb-0310-9956-ffa450edef68
Reported by 河本和彦 kohmoto iris eonet ne jp.


git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1664074 13f79535-47bb-0310-9956-ffa450edef68
are added *after* each other, not before


git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1664406 13f79535-47bb-0310-9956-ffa450edef68
…nd optimize test in insert_compare().

git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1664447 13f79535-47bb-0310-9956-ffa450edef68
…ert_compare() and keep find_compare() in sync.

git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1664451 13f79535-47bb-0310-9956-ffa450edef68
We don't need to create the top before inserting, this will be done
if/once the value is really added (since r1611193).

Check compare value only if m->next is not NULL, otherwise we already
known it is to be handled as negative (down).


git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1664769 13f79535-47bb-0310-9956-ffa450edef68
Generalize the internal stack structure as a queue (FIFO), and use it for the
spare nodes (instead of apr_skiplist_alloc()/free()) and the insertion stack.

Fix a memory leak in destroy() when memory is malloc()ed (pool is NULL).


git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1664775 13f79535-47bb-0310-9956-ffa450edef68
git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1664901 13f79535-47bb-0310-9956-ffa450edef68
git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1664904 13f79535-47bb-0310-9956-ffa450edef68
ylavic and others added 21 commits April 7, 2017 00:01
Axe the 'absolute' argument of apr_{thread,proc,global}_mutex_timedlock()
which was confusing, hence 'timeout' is always relative now.

It still makes sense (to me) to handle a negative timeout as INFINITE, a nul
one as IMMEDIATE, and a positive one as an upper bound timeout (like most if
not all of the underlying system calls...).



git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1790488 13f79535-47bb-0310-9956-ffa450edef68
Make it clear in the type that it's a relative/interval time.



git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1790521 13f79535-47bb-0310-9956-ffa450edef68
apr_proc_mutex_unix_lock_methods_t's timedacquired method.



git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1790523 13f79535-47bb-0310-9956-ffa450edef68
…ivalent

to apr_{thread,proc,global}_trylock(), i.e. immediate attempt to acquire the
lock (but returning APR_TIMEUP if busy).



git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1790632 13f79535-47bb-0310-9956-ffa450edef68
Since proc_pthread_mutex_cond_locked() macro is also used as an lvalue, don't
define it as a conditional and put the condition where needed in the code.



git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1791718 13f79535-47bb-0310-9956-ffa450edef68
Fix proc_mutex_pthread_acquire_ex() for the APR_USE_PROC_PTHREAD_MUTEX_COND
case which shouldn't use undefined pthread_cond_timedwait().



git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1791728 13f79535-47bb-0310-9956-ffa450edef68
…s for

apr_{proc,thread}_{mutex,cond}_timed{lock,wait}(), such that the given timeout
interval value is not truncated or switched from/to signed/unsigned.



git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1792620 13f79535-47bb-0310-9956-ffa450edef68
inherent to some native/OS condvar implementation.



git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1792622 13f79535-47bb-0310-9956-ffa450edef68
Indent block previously preserved (for easier review), no functional change.



git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1792625 13f79535-47bb-0310-9956-ffa450edef68
It can block without timeout in case of EDEADLK.
On Solaris 8 it does not exist, on Solaris 11
it is fixed. For Solaris 10 no patch is available.


git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1792961 13f79535-47bb-0310-9956-ffa450edef68
…y be

inherent to some native/OS condvar implementation.



git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1794266 13f79535-47bb-0310-9956-ffa450edef68
…ompatible

with timed locks, so there is no delta between DEFAULT and DEFAULT_TIMED.

Avoid gratuitous API changes to typical OS lock information.



git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1797415 13f79535-47bb-0310-9956-ffa450edef68
all the tools on any attempt, then fail if any of the tools
are missing/old.



git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1798105 13f79535-47bb-0310-9956-ffa450edef68
In case pfd.fd < 0, the poll() function waits until the time-out is expired.
This problem happens when, for instance, ModSecurity drops a connection in httpd. In this case, ressources gets blocked until the timeout expires.
In this situation, we should immediately return an error.
Same problem/solution in v1.6.x
@marcstern
Copy link
Author

marcstern commented Jun 23, 2017

Credits: @ferrieux

Copy link

@jeking3 jeking3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a good change.

@ylavic
Copy link
Member

ylavic commented Jun 28, 2018

Sorry for the late, gh is not the best place to propose changes for APR, developers mostly look for the dev@apr.apache.org mailing list and the Apache bugzilla.

Regarding the sanity check proposed in this PR, we don't do defensive programming in APR, if apr_wait_for_io_or_timeout() is called with an invalid file or socket fd, please blame the caller.

@ylavic
Copy link
Member

ylavic commented Jun 28, 2018

Oh, already discussed here: https://bz.apache.org/bugzilla/show_bug.cgi?id=61985

asfgit pushed a commit that referenced this pull request Dec 16, 2020
apr_pools: lock parent pool in pool_destroy_debug().

By using apr_pool_clear_debug() instead of pool_clear_debug() in
pool_destroy_debug() we gain the locking provided by the former and thus
protection from concurrent access from apr_pool_walk_tree(), which is
undefined behaviour.

While pool_destroy_debug()=>apr_pool_clear_debug()=>pool_clear_debug() calls
pool_destroy_debug() for all the children pools, this does not cause a deadlock
because apr_pool_clear_debug() locks the parent pool only (not the pool itself)
and thus pool_destroy_debug(pool->child) locks the current pool with no issue.

This fixes use-after-free like the below in httpd (with -D APR_POOL_DEBUG):

=================================================================
==2026856==ERROR: AddressSanitizer: heap-use-after-free on address 0x60600025acf0 at pc 0x7fe738f4c5be bp 0x7fe718598110 sp 0x7fe718598108
READ of size 8 at 0x60600025acf0 thread T51
    #0 0x7fe738f4c5bd in apr_thread_mutex_lock locks/unix/thread_mutex.c:124
    #1 0x7fe738f4e01c in apr_pool_walk_tree memory/unix/apr_pools.c:1505
    #2 0x7fe738f4e066 in apr_pool_walk_tree memory/unix/apr_pools.c:1511
    #3 0x7fe738f4e066 in apr_pool_walk_tree memory/unix/apr_pools.c:1511
    #4 0x7fe738f4e066 in apr_pool_walk_tree memory/unix/apr_pools.c:1511
    #5 0x7fe738f5027c in apr_pool_find memory/unix/apr_pools.c:2291
    #6 0x7fe738f14aba in apr_table_mergen tables/apr_tables.c:746
    #7 0x5578ad926a25 in ap_set_keepalive /home/ylavic/src/apache/httpd/trunk/modules/http/http_protocol.c:309
    #8 0x5578ad93933f in ap_http_header_filter /home/ylavic/src/apache/httpd/trunk/modules/http/http_filters.c:1376
    #9 0x5578ad98f7bd in ap_pass_brigade /home/ylavic/src/apache/httpd/trunk/server/util_filter.c:783
    #10 0x5578ad9a67f3 in ap_content_length_filter /home/ylavic/src/apache/httpd/trunk/server/protocol.c:2046
    #11 0x5578ad98f7bd in ap_pass_brigade /home/ylavic/src/apache/httpd/trunk/server/util_filter.c:783
    #12 0x5578ad9405ae in ap_byterange_filter /home/ylavic/src/apache/httpd/trunk/modules/http/byterange_filter.c:463
    #13 0x5578ad98f7bd in ap_pass_brigade /home/ylavic/src/apache/httpd/trunk/server/util_filter.c:783
    #14 0x7fe7330e398b in ap_headers_output_filter /home/ylavic/src/apache/httpd/trunk/modules/metadata/mod_headers.c:891
    #15 0x5578ad98f7bd in ap_pass_brigade /home/ylavic/src/apache/httpd/trunk/server/util_filter.c:783
    #16 0x7fe732e32dba in session_output_filter /home/ylavic/src/apache/httpd/trunk/modules/session/mod_session.c:501
    #17 0x5578ad98f7bd in ap_pass_brigade /home/ylavic/src/apache/httpd/trunk/server/util_filter.c:783
    #18 0x5578ad9c8ee5 in default_handler /home/ylavic/src/apache/httpd/trunk/server/core.c:5188
    #19 0x5578ad9431bb in ap_run_handler /home/ylavic/src/apache/httpd/trunk/server/config.c:170
    #20 0x5578ad944941 in ap_invoke_handler /home/ylavic/src/apache/httpd/trunk/server/config.c:444
    #21 0x5578ad92cc23 in ap_process_async_request /home/ylavic/src/apache/httpd/trunk/modules/http/http_request.c:463
    #22 0x5578ad924d7c in ap_process_http_async_connection /home/ylavic/src/apache/httpd/trunk/modules/http/http_core.c:158
    #23 0x5578ad925410 in ap_process_http_connection /home/ylavic/src/apache/httpd/trunk/modules/http/http_core.c:252
    #24 0x5578ad97e04d in ap_run_process_connection /home/ylavic/src/apache/httpd/trunk/server/connection.c:42
    #25 0x7fe735c7ef79 in process_socket /home/ylavic/src/apache/httpd/trunk/server/mpm/event/event.c:1097
    #26 0x7fe735c856a0 in worker_thread /home/ylavic/src/apache/httpd/trunk/server/mpm/event/event.c:2386
    #27 0x7fe738f7cef4 in dummy_worker threadproc/unix/thread.c:145
    #28 0x7fe738e3eea6 in start_thread nptl/pthread_create.c:477
    #29 0x7fe738d6ed4e in __clone (/lib/x86_64-linux-gnu/libc.so.6+0xfdd4e)

0x60600025acf0 is located 48 bytes inside of 64-byte region [0x60600025acc0,0x60600025ad00)
freed by thread T63 here:
    #0 0x7fe7391ed277 in __interceptor_free (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x107277)
    #1 0x7fe738f4e9e5 in pool_clear_debug memory/unix/apr_pools.c:1893
    #2 0x7fe738f4ecb2 in pool_destroy_debug memory/unix/apr_pools.c:1956
    #3 0x7fe738f4eeeb in apr_pool_destroy_debug memory/unix/apr_pools.c:2002
    #4 0x5578ada2534b in ap_queue_info_push_pool /home/ylavic/src/apache/httpd/trunk/server/mpm_fdqueue.c:230
    #5 0x7fe735c81412 in process_lingering_close /home/ylavic/src/apache/httpd/trunk/server/mpm/event/event.c:1686
    #6 0x7fe735c7f9bc in process_socket /home/ylavic/src/apache/httpd/trunk/server/mpm/event/event.c:1255
    #7 0x7fe735c856a0 in worker_thread /home/ylavic/src/apache/httpd/trunk/server/mpm/event/event.c:2386
    #8 0x7fe738f7cef4 in dummy_worker threadproc/unix/thread.c:145
    #9 0x7fe738e3eea6 in start_thread nptl/pthread_create.c:477


git-svn-id: https://svn.apache.org/repos/asf/apr/apr/branches/1.7.x@1883751 13f79535-47bb-0310-9956-ffa450edef68
asfgit pushed a commit that referenced this pull request Dec 16, 2020
By using apr_pool_clear_debug() instead of pool_clear_debug() in
pool_destroy_debug() we gain the locking provided by the former and thus
protection from concurrent access from apr_pool_walk_tree(), which is
undefined behaviour.

While pool_destroy_debug()=>apr_pool_clear_debug()=>pool_clear_debug() calls
pool_destroy_debug() for all the children pools, this does not cause a deadlock
because apr_pool_clear_debug() locks the parent pool only (not the pool itself)
and thus pool_destroy_debug(pool->child) locks the current pool with no issue.

This fixes use-after-free like the below in httpd (with -D APR_POOL_DEBUG):

=================================================================
==2026856==ERROR: AddressSanitizer: heap-use-after-free on address 0x60600025acf0 at pc 0x7fe738f4c5be bp 0x7fe718598110 sp 0x7fe718598108
READ of size 8 at 0x60600025acf0 thread T51
    #0 0x7fe738f4c5bd in apr_thread_mutex_lock locks/unix/thread_mutex.c:124
    #1 0x7fe738f4e01c in apr_pool_walk_tree memory/unix/apr_pools.c:1505
    #2 0x7fe738f4e066 in apr_pool_walk_tree memory/unix/apr_pools.c:1511
    #3 0x7fe738f4e066 in apr_pool_walk_tree memory/unix/apr_pools.c:1511
    #4 0x7fe738f4e066 in apr_pool_walk_tree memory/unix/apr_pools.c:1511
    #5 0x7fe738f5027c in apr_pool_find memory/unix/apr_pools.c:2291
    #6 0x7fe738f14aba in apr_table_mergen tables/apr_tables.c:746
    #7 0x5578ad926a25 in ap_set_keepalive /home/ylavic/src/apache/httpd/trunk/modules/http/http_protocol.c:309
    #8 0x5578ad93933f in ap_http_header_filter /home/ylavic/src/apache/httpd/trunk/modules/http/http_filters.c:1376
    #9 0x5578ad98f7bd in ap_pass_brigade /home/ylavic/src/apache/httpd/trunk/server/util_filter.c:783
    #10 0x5578ad9a67f3 in ap_content_length_filter /home/ylavic/src/apache/httpd/trunk/server/protocol.c:2046
    #11 0x5578ad98f7bd in ap_pass_brigade /home/ylavic/src/apache/httpd/trunk/server/util_filter.c:783
    #12 0x5578ad9405ae in ap_byterange_filter /home/ylavic/src/apache/httpd/trunk/modules/http/byterange_filter.c:463
    #13 0x5578ad98f7bd in ap_pass_brigade /home/ylavic/src/apache/httpd/trunk/server/util_filter.c:783
    #14 0x7fe7330e398b in ap_headers_output_filter /home/ylavic/src/apache/httpd/trunk/modules/metadata/mod_headers.c:891
    #15 0x5578ad98f7bd in ap_pass_brigade /home/ylavic/src/apache/httpd/trunk/server/util_filter.c:783
    #16 0x7fe732e32dba in session_output_filter /home/ylavic/src/apache/httpd/trunk/modules/session/mod_session.c:501
    #17 0x5578ad98f7bd in ap_pass_brigade /home/ylavic/src/apache/httpd/trunk/server/util_filter.c:783
    #18 0x5578ad9c8ee5 in default_handler /home/ylavic/src/apache/httpd/trunk/server/core.c:5188
    #19 0x5578ad9431bb in ap_run_handler /home/ylavic/src/apache/httpd/trunk/server/config.c:170
    #20 0x5578ad944941 in ap_invoke_handler /home/ylavic/src/apache/httpd/trunk/server/config.c:444
    #21 0x5578ad92cc23 in ap_process_async_request /home/ylavic/src/apache/httpd/trunk/modules/http/http_request.c:463
    #22 0x5578ad924d7c in ap_process_http_async_connection /home/ylavic/src/apache/httpd/trunk/modules/http/http_core.c:158
    #23 0x5578ad925410 in ap_process_http_connection /home/ylavic/src/apache/httpd/trunk/modules/http/http_core.c:252
    #24 0x5578ad97e04d in ap_run_process_connection /home/ylavic/src/apache/httpd/trunk/server/connection.c:42
    #25 0x7fe735c7ef79 in process_socket /home/ylavic/src/apache/httpd/trunk/server/mpm/event/event.c:1097
    #26 0x7fe735c856a0 in worker_thread /home/ylavic/src/apache/httpd/trunk/server/mpm/event/event.c:2386
    #27 0x7fe738f7cef4 in dummy_worker threadproc/unix/thread.c:145
    #28 0x7fe738e3eea6 in start_thread nptl/pthread_create.c:477
    #29 0x7fe738d6ed4e in __clone (/lib/x86_64-linux-gnu/libc.so.6+0xfdd4e)

0x60600025acf0 is located 48 bytes inside of 64-byte region [0x60600025acc0,0x60600025ad00)
freed by thread T63 here:
    #0 0x7fe7391ed277 in __interceptor_free (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x107277)
    #1 0x7fe738f4e9e5 in pool_clear_debug memory/unix/apr_pools.c:1893
    #2 0x7fe738f4ecb2 in pool_destroy_debug memory/unix/apr_pools.c:1956
    #3 0x7fe738f4eeeb in apr_pool_destroy_debug memory/unix/apr_pools.c:2002
    #4 0x5578ada2534b in ap_queue_info_push_pool /home/ylavic/src/apache/httpd/trunk/server/mpm_fdqueue.c:230
    #5 0x7fe735c81412 in process_lingering_close /home/ylavic/src/apache/httpd/trunk/server/mpm/event/event.c:1686
    #6 0x7fe735c7f9bc in process_socket /home/ylavic/src/apache/httpd/trunk/server/mpm/event/event.c:1255
    #7 0x7fe735c856a0 in worker_thread /home/ylavic/src/apache/httpd/trunk/server/mpm/event/event.c:2386
    #8 0x7fe738f7cef4 in dummy_worker threadproc/unix/thread.c:145
    #9 0x7fe738e3eea6 in start_thread nptl/pthread_create.c:477


git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1883750 13f79535-47bb-0310-9956-ffa450edef68
asfgit pushed a commit that referenced this pull request Sep 10, 2021
This can happen in cprng_stream_ctx_make() on error paths, or at thread exit
with APR_CRYPTO_PRNG_PER_THREAD like the below.

Direct leak of 64 byte(s) in 8 object(s) allocated from:
    #0 0x7efd954c7628 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x107628)
    #1 0x7efd921db6ca  (<unknown module>)
    #2 0x7efd952937a2 in apr_crypto_prng_create crypto/apr_crypto_prng.c:367
    #3 0x7efd95292c1e in apr_crypto_random_thread_bytes crypto/apr_crypto_prng.c:218
    #4 0x5611dbbb9440 in thread_func /home/yle/src/apache/apr/trunk.ro/test/testcrypto.c:2597
    #5 0x7efd9537dd86 in dummy_worker threadproc/unix/thread.c:148
    #6 0x7efd951efea6 in start_thread nptl/pthread_create.c:477



git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1893201 13f79535-47bb-0310-9956-ffa450edef68
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet