ciniml / rust-dap

CMSIS-DAP Rust implementation
Apache License 2.0
88 stars 10 forks source link

Possible denial of service by UART data transmission #51

Open Dblm0 opened 1 year ago

Dblm0 commented 1 year ago

Thanks for the firmware! I'm using it on rpi_pico board with swd and set_clock features enabled. During exploitation, I ran over a problem, when a probe stops servicing.

To reproduce I'm continuously transferring small amount of random bytes and for the short period of time the device stops any exchange over USB.

From my debugging observations the firmware comes to some state, when only uart_irq task is being triggered over and over, even if there is no data transfer. I assume, it might be rtic problem, but I have no experience with it to do proper diagnostics.

I've also tried increasing UART_RX_QUEUE_SIZE without any luck.

Dblm0 commented 1 year ago

Just noticed, UART0_IRQ task have higher priority than USBCTRL_IRQ. So, once UART FIFO is overflown, none of the remaining tasks could be scheduled anymore.

I suggest swapping UART0_IRQ and USBCTRL_IRQ task priorities or even making them the same.

ciniml commented 1 year ago

Thank you for reporting the issue and sorry for late response.

I could not reproduce the issue in the release build, but in the debug build, I confirmed that the firmware hangs when sending README.md to the USB CDC device repeatedly.

$ for i in `seq 0 100`; do cat README.md > /dev/ttyACM0; done

I also confirmed that it does not reproduce anymore by replacing the priorities of USBCTRL_IRQ and UART0_IRQ.

Dblm0 commented 1 year ago

Sorry for misleading, but giving USBCTRL_IRQ the highest priority could lead to a different issue in case of high DAP traffic. I ended up setting priorities to the same value and setting small value to both spsc's, like 32 or even smaller. It solves the current issue, but isn't ideal.

For testing purposes I use usb-uart adapter and send UART data in both directions, checking for data consistency at the end.