laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/42162?usp=email )
Change subject: main: don't access msgbs after giving them away ......................................................................
main: don't access msgbs after giving them away
Because of IRQ, it is dangerous to access a msgb, which has been giving away to a queue (ownership transfer).
Further OSMO_ASSERT() should never fail on a queue'd object, except it has been taken already from an IRQ context. Fix a potential crash.
Fixes: a684bc4e38b4 ("Make ch9 usb tests work") Change-Id: I79844f77d5cd75e08b0eb44b22c4ad223cb79dcb --- M sysmoOCTSIM/main.c 1 file changed, 0 insertions(+), 30 deletions(-)
Approvals: laforge: Looks good to me, approved Jenkins Builder: Verified
diff --git a/sysmoOCTSIM/main.c b/sysmoOCTSIM/main.c index ce4d4e6..92c5bbd 100644 --- a/sysmoOCTSIM/main.c +++ b/sysmoOCTSIM/main.c @@ -199,8 +199,6 @@ if (rc != ERR_NONE) { // ep_q->in_progress = msg; // msgb_enqueue_irqsafe(&ep_q->list, msg); - // OSMO_ASSERT(msg->list.next != LLIST_POISON1) - // OSMO_ASSERT(msg->list.prev != LLIST_POISON2) printf("EP %s failed: %d\r\n", ep_q->name, rc); return -1; } @@ -226,10 +224,6 @@ if (rc != ERR_NONE) { /* re-add to the list of free msgb's */ llist_add_tail_at(&msg->list, &g_ccid_s.free_q); - OSMO_ASSERT(msg->list.next != LLIST_POISON1) - OSMO_ASSERT(msg->list.next->next != LLIST_POISON1) - OSMO_ASSERT(msg->list.prev != LLIST_POISON2) - OSMO_ASSERT(msg->list.prev->prev != LLIST_POISON2) return 0; } return 1; @@ -262,10 +256,6 @@ msgb_put(msg, transferred); /* append to list of pending-to-be-handed messages */ llist_add_tail_at(&msg->list, &g_ccid_s.out_ep.list); - OSMO_ASSERT(msg->list.next != LLIST_POISON1) - OSMO_ASSERT(msg->list.next->next != LLIST_POISON1) - OSMO_ASSERT(msg->list.prev != LLIST_POISON2) - OSMO_ASSERT(msg->list.prev->prev != LLIST_POISON2) g_ccid_s.out_ep.in_progress = NULL;
if(code != USB_XFER_DONE) @@ -283,10 +273,6 @@ if (msg) { /* return the message back to the queue of free message buffers */ llist_add_tail_at(&msg->list, &g_ccid_s.free_q); - OSMO_ASSERT(msg->list.next != LLIST_POISON1) - OSMO_ASSERT(msg->list.next->next != LLIST_POISON1) - OSMO_ASSERT(msg->list.prev != LLIST_POISON2) - OSMO_ASSERT(msg->list.prev->prev != LLIST_POISON2) g_ccid_s.in_ep.in_progress = NULL; }
@@ -308,10 +294,6 @@ if (msg) { /* return the message back to the queue of free message buffers */ llist_add_tail_at(&msg->list, &g_ccid_s.free_q); - OSMO_ASSERT(msg->list.next != LLIST_POISON1) - OSMO_ASSERT(msg->list.next->next != LLIST_POISON1) - OSMO_ASSERT(msg->list.prev != LLIST_POISON2) - OSMO_ASSERT(msg->list.prev->prev != LLIST_POISON2) g_ccid_s.irq_ep.in_progress = NULL; }
@@ -513,10 +495,6 @@
/* append to list of pending-to-be-handed messages */ llist_add_tail_at(&msg->list, &g_ccid_s.in_ep.list); - OSMO_ASSERT(msg->list.next != LLIST_POISON1) - OSMO_ASSERT(msg->list.next->next != LLIST_POISON1) - OSMO_ASSERT(msg->list.prev != LLIST_POISON2) - OSMO_ASSERT(msg->list.prev->prev != LLIST_POISON2) submit_next_in(); return 0; } @@ -578,10 +556,6 @@ while ((msg = msgb_dequeue_irqsafe(&cur_epq->list))) { msgb_reset(msg); llist_add_tail_at(&msg->list, &g_ccid_s.free_q); - OSMO_ASSERT(msg->list.next != LLIST_POISON1) - OSMO_ASSERT(msg->list.next->next != LLIST_POISON1) - OSMO_ASSERT(msg->list.prev != LLIST_POISON2) - OSMO_ASSERT(msg->list.prev->prev != LLIST_POISON2) } #if 0 struct msgb *cur_msg = cur_epq->in_progress; @@ -593,10 +567,6 @@
msgb_reset(cur_msg); llist_add_tail_at(&cur_msg->list, &g_ccid_s.free_q); - OSMO_ASSERT(msg->list.next != LLIST_POISON1) - OSMO_ASSERT(msg->list.next->next != LLIST_POISON1) - OSMO_ASSERT(msg->list.prev != LLIST_POISON2) - OSMO_ASSERT(msg->list.prev->prev != LLIST_POISON2) cur_epq->in_progress = NULL; } #endif