You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by as...@apache.org on 2022/02/22 23:30:19 UTC
[qpid-proton] branch main updated: PROTON-2503: Stop ignoring some received framing errors
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 <as...@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-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org