This is an automated email from the ASF dual-hosted git repository. astitcher pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/qpid-proton.git
The following commit(s) were added to refs/heads/main by this push: new 9976249 PROTON-2503: Stop ignoring some received framing errors 9976249 is described below commit 9976249e8b17b5003529a04deae66c3c8345f8da Author: Andrew Stitcher <astitc...@apache.org> AuthorDate: Tue Feb 22 18:02:01 2022 -0500 PROTON-2503: Stop ignoring some received framing errors This bug was discovered by the OSS fuzz project --- c/src/core/consumers.h | 6 +-- c/src/proactor/epoll-internal.h | 2 +- c/src/proactor/epoll.c | 52 ++++++++++++--------- c/src/sasl/sasl.c | 4 +- .../crash/leak-5052013914750976 | Bin 0 -> 114 bytes python/setup.py.in | 3 -- 6 files changed, 36 insertions(+), 31 deletions(-) diff --git a/c/src/core/consumers.h b/c/src/core/consumers.h index b4140e8..2f20cba 100644 --- a/c/src/core/consumers.h +++ b/c/src/core/consumers.h @@ -539,13 +539,13 @@ static inline bool consume_descriptor(pni_consumer_t* consumer, pni_consumer_t * if (!pni_consumer_readf8(consumer, &type)) return false; switch (type) { case PNE_DESCRIPTOR: { - bool lq = consume_ulong(consumer, descriptor); + if (!consume_ulong(consumer, descriptor)) return false; size_t sposition = consumer->position; uint8_t type; - consume_single_value_not_described(consumer, &type); + if (!consume_single_value_not_described(consumer, &type)) return false; size_t scsize = consumer->position > sposition ? consumer->position-sposition : 0; *subconsumer = (pni_consumer_t){.output_start=consumer->output_start+sposition, .position=0, .size=scsize}; - return lq; + return true; } default: pni_consumer_skip_value_not_described(consumer, type); diff --git a/c/src/proactor/epoll-internal.h b/c/src/proactor/epoll-internal.h index 79dddaa..8db12a1 100644 --- a/c/src/proactor/epoll-internal.h +++ b/c/src/proactor/epoll-internal.h @@ -192,7 +192,7 @@ struct pn_proactor_t { tslot_t *last_earmark; task_t *sched_ready_first; task_t *sched_ready_last; - bool sched_ready_pending; + task_t *sched_ready_current; // TODO: remove or use for sceduling priority or fairness unsigned int sched_ready_count; task_t *resched_first; task_t *resched_last; diff --git a/c/src/proactor/epoll.c b/c/src/proactor/epoll.c index 1ff68ef..19867af 100644 --- a/c/src/proactor/epoll.c +++ b/c/src/proactor/epoll.c @@ -250,8 +250,8 @@ void task_init(task_t *tsk, task_type_t t, pn_proactor_t *p) { * are needed to cross or reconcile the two portions of the list. */ -// Call with sched lock held and sched_ready_count > 0. -static task_t *sched_ready_pop_front(pn_proactor_t *p) { +// Call with sched lock held. +static void pop_ready_task(task_t *tsk) { // every task on the sched_ready_list is either currently running, // or to be scheduled. schedule() will not "see" any of the ready_next // pointers until ready and working have transitioned to 0 @@ -262,19 +262,22 @@ static task_t *sched_ready_pop_front(pn_proactor_t *p) { // !ready .. schedule() .. on ready_list .. on sched_ready_list .. working task .. !sched_ready && !ready // // Intervening locks at each transition ensures ready_next has memory coherence throughout the ready task scheduling cycle. + // TODO: sched_ready list changed to sequential processing. Review need for sched_ready_current. + pn_proactor_t *p = tsk->proactor; + if (tsk == p->sched_ready_current) + p->sched_ready_current = tsk->ready_next; + assert (tsk == p->sched_ready_first); assert (p->sched_ready_count); - task_t *tsk = p->sched_ready_first; p->sched_ready_count--; if (tsk == p->sched_ready_last) { p->sched_ready_first = p->sched_ready_last = NULL; } else { p->sched_ready_first = tsk->ready_next; } - if (p->sched_ready_count == 0) { - assert(!p->sched_ready_first); - p->sched_ready_pending = false; + if (!p->sched_ready_first) { + p->sched_ready_last = NULL; + assert(p->sched_ready_count == 0); } - return tsk; } // Call only as the poller task that has already called schedule_ready_list() and already @@ -2262,20 +2265,21 @@ static pn_event_batch_t *process(task_t *tsk) { // Call with both sched_mutex and eventfd_mutex held static void schedule_ready_list(pn_proactor_t *p) { - // Append ready_list_first..ready_list_last to end of sched_ready_last - // May see several in single do_epoll() if EINTR. + // append ready_list_first..ready_list_last to end of sched_ready_last if (p->ready_list_first) { if (p->sched_ready_last) p->sched_ready_last->ready_next = p->ready_list_first; // join them if (!p->sched_ready_first) p->sched_ready_first = p->ready_list_first; p->sched_ready_last = p->ready_list_last; + if (!p->sched_ready_current) + p->sched_ready_current = p->sched_ready_first; p->ready_list_first = p->ready_list_last = NULL; - - // Track sched_ready_count to know how many threads may be needed. - p->sched_ready_count += p->ready_list_count; - p->ready_list_count = 0; } + + // Track sched_ready_count to know how many threads may be needed. + p->sched_ready_count = p->ready_list_count; + p->ready_list_count = 0; } // Call with schedule lock and eventfd lock held. Called only by poller thread. @@ -2398,14 +2402,17 @@ static task_t *next_runnable(pn_proactor_t *p, tslot_t *ts) { } } - // sched_ready list tasks deferred in poller_do_epoll() - while (p->sched_ready_pending) { - tsk = sched_ready_pop_front(p); + // rest of sched_ready list + while (p->sched_ready_count) { + tsk = p->sched_ready_current; assert(tsk->ready); // eventfd_mutex required post ready set and pre move to sched_ready_list if (post_ready(p, tsk)) { + pop_ready_task(tsk); // updates sched_ready_current assert(!tsk->runnables_idx && !tsk->runner); assign_thread(ts, tsk); return tsk; + } else { + pop_ready_task(tsk); } } @@ -2493,7 +2500,7 @@ static pn_event_batch_t *next_event_batch(pn_proactor_t* p, bool can_block) { static bool poller_do_epoll(struct pn_proactor_t* p, tslot_t *ts, bool can_block) { // As poller with lots to do, be mindful of hogging the sched lock. Release when making kernel calls. assert(!p->resched_cutoff); - assert(!p->sched_ready_first && !p->sched_ready_pending); + assert(!p->sched_ready_first); int n_events; task_t *tsk; bool unpolled_work = false; @@ -2600,20 +2607,21 @@ static bool poller_do_epoll(struct pn_proactor_t* p, tslot_t *ts, bool can_block if (warm_tries < 0) warm_tries = 0; + task_t *ctsk = p->sched_ready_current; int max_runnables = p->runnables_capacity; while (p->sched_ready_count && p->n_runnables < max_runnables && warm_tries) { - task_t *ctsk = sched_ready_pop_front(p); + assert(ctsk); tsk = post_ready(p, ctsk); + pop_ready_task(ctsk); warm_tries--; if (tsk) make_runnable(tsk); + ctsk = ctsk->ready_next; } - // sched_ready list is now either consumed or partially deferred. - // Allow next_runnable() to see any remaining sched_ready tasks. - p->sched_ready_pending = p->sched_ready_count > 0; + p->sched_ready_current = ctsk; while (p->resched_cutoff && p->n_runnables < max_runnables && warm_tries) { - task_t *ctsk = resched_pop_front(p); + ctsk = resched_pop_front(p); assert(ctsk->runner == RESCHEDULE_PLACEHOLDER && !ctsk->runnables_idx); ctsk->runner = NULL; // Allow task to run again. warm_tries--; diff --git a/c/src/sasl/sasl.c b/c/src/sasl/sasl.c index 09a3496..1fc16a9 100644 --- a/c/src/sasl/sasl.c +++ b/c/src/sasl/sasl.c @@ -946,7 +946,7 @@ int pn_do_mechanisms(pn_transport_t *transport, uint8_t frame_type, uint16_t cha switch (element_type) { case PNE_SYM8: while (element_count) { - pni_consumer_readv8(&subconsumer, &symbol); + if (!pni_consumer_readv8(&subconsumer, &symbol)) break; if (pni_sasl_client_included_mech(sasl->included_mechanisms, symbol)) { pn_string_addf(mechs, "%.*s ", (int)symbol.size, symbol.start); } @@ -955,7 +955,7 @@ int pn_do_mechanisms(pn_transport_t *transport, uint8_t frame_type, uint16_t cha break; case PNE_SYM32: while (element_count) { - pni_consumer_readv32(&subconsumer, &symbol); + if (!pni_consumer_readv32(&subconsumer, &symbol)) break; if (pni_sasl_client_included_mech(sasl->included_mechanisms, symbol)) { pn_string_addf(mechs, "%.*s ", (int)symbol.size, symbol.start); } diff --git a/c/tests/fuzz/fuzz-connection-driver/crash/leak-5052013914750976 b/c/tests/fuzz/fuzz-connection-driver/crash/leak-5052013914750976 new file mode 100644 index 0000000..1c9e3df Binary files /dev/null and b/c/tests/fuzz/fuzz-connection-driver/crash/leak-5052013914750976 differ diff --git a/python/setup.py.in b/python/setup.py.in index 3412d4a..697fa70 100644 --- a/python/setup.py.in +++ b/python/setup.py.in @@ -125,7 +125,6 @@ class Configure(build_ext): # Look for any optional libraries that proton needs, and adjust the # source list and compile flags as necessary. - library_dirs = [] libraries = [] includes = [] macros = [] @@ -146,7 +145,6 @@ class Configure(build_ext): # pkg-config for a minimum version 0. If it's installed, it should # return True and we'll use it. Otherwise, we'll use the stub. if misc.pkg_config_version_installed('openssl', atleast='0'): - library_dirs += [misc.pkg_config_get_var('openssl', 'libdir')] libraries += ['ssl', 'crypto'] includes += [misc.pkg_config_get_var('openssl', 'includedir')] sources.append(os.path.join(proton_src, 'ssl', 'openssl.c')) @@ -216,7 +214,6 @@ class Configure(build_ext): # lastly replace the libqpid-proton-core dependency with libraries required # by the Proton objects: - _cproton.library_dirs = library_dirs _cproton.libraries = libraries def libqpid_proton_installed(self, version): --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@qpid.apache.org For additional commands, e-mail: commits-h...@qpid.apache.org