Hoernchen has uploaded this change for review. (
https://gerrit.osmocom.org/c/simtrace2/+/43127?usp=email )
Change subject: firmware: protect uart_tx_queue against ISR
......................................................................
firmware: protect uart_tx_queue against ISR
cardem:
- dispatch_usb_command_cardem() appends to uart_tx_queue from the main loop
- tx_byte_tpdu() dequeues from the USART IRQ handler @ NVIC prio 0
card_handle_reset() has the same issue, drains queue and
frees uart_tx_msg from main loop while the ISR may own them.
All of this needs protection against the irq.
Needs a fixed llist_add_tail_irqsafe(), which called __enable_irq() instead of
restoring the saved PRIMASK for some unknown reason?!?!?!?
Change-Id: I7d9cdcc56263b27dfd4649dfb1da1d67761ee923
---
M firmware/libcommon/include/llist_irqsafe.h
M firmware/libcommon/source/card_emu.c
M firmware/libcommon/source/mode_cardemu.c
3 files changed, 21 insertions(+), 8 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/simtrace2 refs/changes/27/43127/1
diff --git a/firmware/libcommon/include/llist_irqsafe.h
b/firmware/libcommon/include/llist_irqsafe.h
index 8aafcfb..e6c893a 100644
--- a/firmware/libcommon/include/llist_irqsafe.h
+++ b/firmware/libcommon/include/llist_irqsafe.h
@@ -34,7 +34,7 @@
local_irq_save(x);
llist_add_tail(_new, head);
- __enable_irq();
+ local_irq_restore(x);
}
static inline struct llist_head *llist_head_dequeue_irqsafe(struct llist_head
*head)
diff --git a/firmware/libcommon/source/card_emu.c
b/firmware/libcommon/source/card_emu.c
index 31ba79c..98ce662 100644
--- a/firmware/libcommon/source/card_emu.c
+++ b/firmware/libcommon/source/card_emu.c
@@ -219,19 +219,31 @@
static void card_handle_reset(struct card_handle *ch)
{
struct msgb *msg;
+ unsigned long x;
card_emu_uart_update_wt(ch->uart_chan, 0);
- /* release any buffers we may still own */
- if (ch->uart_tx_msg) {
- usb_buf_free(ch->uart_tx_msg);
- ch->uart_tx_msg = NULL;
- }
+ /* Release any buffers we may still own.
+ * uart_tx_msg + uart_tx_queue are shared with the UART IRQ handler,
+ * that preempts us here -> needs atomic detach and free */
+ local_irq_save(x);
+ msg = ch->uart_tx_msg;
+ ch->uart_tx_msg = NULL;
+ local_irq_restore(x);
+ if (msg)
+ usb_buf_free(msg);
+
if (ch->uart_rx_msg) {
usb_buf_free(ch->uart_rx_msg);
ch->uart_rx_msg = NULL;
}
- while ((msg = msgb_dequeue(&ch->uart_tx_queue))) {
+
+ while (1) {
+ local_irq_save(x);
+ msg = msgb_dequeue(&ch->uart_tx_queue);
+ local_irq_restore(x);
+ if (!msg)
+ break;
usb_buf_free(msg);
}
}
diff --git a/firmware/libcommon/source/mode_cardemu.c
b/firmware/libcommon/source/mode_cardemu.c
index 77f7454..a0014d2 100644
--- a/firmware/libcommon/source/mode_cardemu.c
+++ b/firmware/libcommon/source/mode_cardemu.c
@@ -737,7 +737,8 @@
switch (hdr->msg_type) {
case SIMTRACE_MSGT_DT_CEMU_TX_DATA:
queue = card_emu_get_uart_tx_queue(ci->ch);
- llist_add_tail(&msg->list, queue);
+ /* drained from the USART IRQ handler at highest NVIC prio */
+ llist_add_tail_irqsafe(&msg->list, queue);
card_emu_have_new_uart_tx(ci->ch);
break;
case SIMTRACE_MSGT_DT_CEMU_SET_ATR:
--
To view, visit https://gerrit.osmocom.org/c/simtrace2/+/43127?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: simtrace2
Gerrit-Branch: master
Gerrit-Change-Id: I7d9cdcc56263b27dfd4649dfb1da1d67761ee923
Gerrit-Change-Number: 43127
Gerrit-PatchSet: 1
Gerrit-Owner: Hoernchen <[email protected]>
