sys/usb/cdc_acm: fix garbage input when disconnected #21890

benpicco · 2025-11-17T20:09:56Z

Contribution description

usbus_cdc_acm tries to buffer output while disconnected, but fails hard at this.

Testing procedure

Enable the stdio_cdc_acm on any application that comes with a shell.
I added an output of the received data to cdc_acm_stdio.c:

--- i/sys/usb/usbus/cdc/acm/cdc_acm_stdio.c
+++ w/sys/usb/usbus/cdc/acm/cdc_acm_stdio.c
@@ -51,6 +51,9 @@ static void _cdc_acm_rx_pipe(usbus_cdcacm_device_t *cdcacm,
                              uint8_t *data, size_t len)
 {
     (void)cdcacm;
+    data[len] = 0;
+    printf("got %u bytes '%s'\n", len, (char *)data);
+
     for (size_t i = 0; i < len; i++) {
         isrpipe_write_one(&stdin_isrpipe, data[i]);
     }

master

2025-11-17 21:09:01,404 # Reconnected to serial port /dev/ttyACM1
2025-11-17 21:09:01,405 # got 1 bytes ':'
2025-11-17 21:09:01,405 # got 1 bytes 'T'
2025-11-17 21:09:01,405 # got 1 bytes 'h'
2025-11-17 21:09:01,405 # got 1 bytes 's'
2025-11-17 21:09:01,405 # got 1 bytes 'i'
2025-11-17 21:09:01,406 # got 1 bytes ' '
2025-11-17 21:09:01,406 # got 1 bytes 'I'
ps
2025-11-17 21:09:05,552 # i(:Thsi Igot 3 bytes 'ps
2025-11-17 21:09:05,552 # '
2025-11-17 21:09:05,552 # ps
2025-11-17 21:09:05,553 # shell: command not found: i(:Thsi

with this patch

2025-11-17 21:06:54,685 # Reconnected to serial port /dev/ttyACM1
ps
2025-11-17 21:07:45,989 # got 3 bytes 'ps
2025-11-17 21:07:45,989 # '
2025-11-17 21:07:45,989 # ps
2025-11-17 21:07:45,990 # 	pid | name                 | state    Q | pri | stack  ( used) ( free) | base addr  | current     
2025-11-17 21:07:45,991 # 	  - | isr_stack            | -        - |   - |    512 (  224) (  288) | 0x20000000 | 0x200001c8
2025-11-17 21:07:45,991 # 	  1 | main                 | running  Q |   7 |   1536 (  764) (  772) | 0x200008c8 | 0x20000d14 
2025-11-17 21:07:45,992 # 	  2 | pktdump              | bl rx    _ |   6 |   1472 (  172) ( 1300) | 0x200036d0 | 0x20003be4 
2025-11-17 21:07:45,993 # 	  3 | 6lo                  | bl rx    _ |   3 |    960 (  264) (  696) | 0x200046f0 | 0x200049f4 
2025-11-17 21:07:45,993 # 	  4 | ipv6                 | bl rx    _ |   4 |    960 (  372) (  588) | 0x2000100c | 0x20001304 
2025-11-17 21:07:45,995 # 	  5 | udp                  | bl rx    _ |   5 |    448 (  192) (  256) | 0x20004af4 | 0x20004bf4 
2025-11-17 21:07:45,996 # 	  6 | usbus                | bl anyfl _ |   1 |   1024 (  696) (  328) | 0x200004c8 | 0x2000074c 
2025-11-17 21:07:45,997 # 	  7 | nrf802154            | bl anyfl _ |   2 |    896 (  304) (  592) | 0x20001a60 | 0x20001d24 
2025-11-17 21:07:45,997 # 	  8 | RPL                  | bl rx    _ |   5 |   1024 (  204) (  820) | 0x20003ed8 | 0x2000420c 
2025-11-17 21:07:45,997 # 	    | SUM                  |            |     |   8832 ( 3192) ( 5640)

Issues/PRs references

Fixes #20305.
Fixes #20544.

riot-ci · 2025-11-17T20:22:58Z

Murdock results

✔️ PASSED

494e171 sys/usb_cdc_acm_stdio: use isrpipe_write()

Success	Failures	Total	Runtime
10930	0	10931	09m:12s

Artifacts

Documentation preview

maribu · 2025-11-17T20:23:56Z

Does this fix #20544 ?

crasbe · 2025-11-17T23:33:54Z

Does this fix #20544 ?

It certainly fixes #20305, I tried it with a Feather-M0. I'll try to check with an nRF52840DK tomorrow.

Behavior with master:

cbuec@W11nMate:~/RIOTstuff/riot-vanilla/RIOT$ BOARD=feather-m0 make -C tests/sys/shell term
make: Entering directory '/home/cbuec/RIOTstuff/riot-vanilla/RIOT/tests/sys/shell'
/home/cbuec/RIOTstuff/riot-vanilla/RIOT/dist/tools/pyterm/pyterm -p "/dev/ttyACM0" -b "115200" -ln "/tmp/pyterm-cbuec" -rn "2025-11-18_00.31.20-tests_shell-feather-m0"
2025-11-18 00:31:20,396 # Connect to serial port /dev/ttyACM0
Welcome to pyterm!
Type '/exit' to exit.
help
2025-11-18 00:31:22,540 # main(): This is help
2025-11-18 00:31:22,540 # shell: command not found: main():
>

Behavior with this PR:

cbuec@W11nMate:~/RIOTstuff/riot-vanilla/RIOT$ BOARD=feather-m0 make -C tests/sys/shell term
make: Entering directory '/home/cbuec/RIOTstuff/riot-vanilla/RIOT/tests/sys/shell'
/home/cbuec/RIOTstuff/riot-vanilla/RIOT/dist/tools/pyterm/pyterm -p "/dev/ttyACM0" -b "115200" -ln "/tmp/pyterm-cbuec" -rn "2025-11-18_00.29.55-tests_shell-feather-m0"
2025-11-18 00:29:55,929 # Connect to serial port /dev/ttyACM0
Welcome to pyterm!
Type '/exit' to exit.
help
2025-11-18 00:30:00,008 # help
2025-11-18 00:30:00,010 # Command              Description
2025-11-18 00:30:00,011 # ---------------------------------------
2025-11-18 00:30:00,012 # bufsize              Get the shell's buffer size
2025-11-18 00:30:00,013 # start_test           starts a test
2025-11-18 00:30:00,014 # end_test             ends a test
2025-11-18 00:30:00,016 # echo                 prints the input command
2025-11-18 00:30:00,018 # empty                print nothing on command
2025-11-18 00:30:00,019 # periodic             periodically print command
2025-11-18 00:30:00,020 # xfa_test1            xfa test command 1
2025-11-18 00:30:00,021 # xfa_test2            xfa test command 2
2025-11-18 00:30:00,022 # app_metadata         Returns application metadata
2025-11-18 00:30:00,023 # pm                   interact with layered PM subsystem
2025-11-18 00:30:00,024 # ps                   Prints information about running threads.
2025-11-18 00:30:00,025 # version              Prints current RIOT_VERSION
2025-11-18 00:30:00,028 # bootloader           Reboot to bootloader
2025-11-18 00:30:00,029 # reboot               Reboot the node
>

I had to create an UF2 file because the Feather-M0 and WSL don't really play together that well.

crasbe · 2025-11-17T23:35:19Z

It might therefore also fix #21231.

crasbe · 2025-11-17T23:45:34Z

No, it does not fix #21231:

cbuec@W11nMate:~/RIOTstuff/riot-vanilla/RIOT$ git status
On branch usbus/cdc/acm-fix_garbage
nothing to commit, working tree clean
cbuec@W11nMate:~/RIOTstuff/riot-vanilla/RIOT$ BOARD=seeedstudio-xiao-nrf52840-sense make -C tests/sys/pthread_flood/ test
make: Entering directory '/home/cbuec/RIOTstuff/riot-vanilla/RIOT/tests/sys/pthread_flood'
r
/home/cbuec/RIOTstuff/riot-vanilla/RIOT/dist/tools/pyterm/pyterm -p "/dev/ttyACM0" -b "115200" -ln "/tmp/pyterm-cbuec" -rn "2025-11-18_00.44.16-tests_pthread_flood-seeedstudio-xiao-nrf52840-sense" --no-reconnect --noprefix --no-repeat-command-on-empty-line
Connect to serial port /dev/ttyACM0
Welcome to pyterm!
Type '/exit' to exit.
READY
s
START
main(): This is RIOT! (Version: 2026.01-devel-138-g494e1-usbus/cdc/acm-fix_garbage)
Spawning pthreads
......
created 6 pthreads
created 6 threads
wait for created pthreads to exit...
{ "threads": [{ "name": "pthread", "stack_size": 256, "stack_used": 128}]}
{ "threads": [{ "name": "pthread", "stack_size": 256, "stack_used": 128}]}
{ "threads": [{ "name": "pthread", "stack_size": 256, "stack_used": 128}]}
Timeout in expect script at "child.expect_exact("SUCCESS")" (tests/sys/pthread_flood/tests/01-run.py:14)

make: *** [/home/cbuec/RIOTstuff/riot-vanilla/RIOT/makefiles/tests/tests.inc.mk:32: test] Error 1
make: Leaving directory '/home/cbuec/RIOTstuff/riot-vanilla/RIOT/tests/sys/pthread_flood'

crasbe · 2025-11-17T23:57:33Z

Does this fix #20544 ?

It certainly fixes #20305, I tried it with a Feather-M0. I'll try to check with an nRF52840DK tomorrow.

I remembered that I have a Seeedstudio Xiao nRF52840 Sense laying around, which is also based on the nRF52840 as is the Arduino Nano 33 BLE.
Therefore I'd dare to say it also fixes that issue.

Behavior on master:

cbuec@W11nMate:~/RIOTstuff/riot-vanilla/RIOT$ BOARD=seeedstudio-xiao-nrf52840-sense make -C tests/sys/shell term
make: Entering directory '/home/cbuec/RIOTstuff/riot-vanilla/RIOT/tests/sys/shell'
/home/cbuec/RIOTstuff/riot-vanilla/RIOT/dist/tools/pyterm/pyterm -p "/dev/ttyACM0" -b "115200" -ln "/tmp/pyterm-cbuec" -rn "2025-11-18_00.38.42-tests_shell-seeedstudio-xiao-nrf52840-sense"
2025-11-18 00:38:42,746 # Connect to serial port /dev/ttyACM0
Welcome to pyterm!
Type '/exit' to exit.
help
2025-11-18 00:38:59,877 # main(): This is help
2025-11-18 00:38:59,881 # shell: command not found: main():

Behavior with this PR:

cbuec@W11nMate:~/RIOTstuff/riot-vanilla/RIOT$ BOARD=seeedstudio-xiao-nrf52840-sense make -C tests/sys/shell term
make: Entering directory '/home/cbuec/RIOTstuff/riot-vanilla/RIOT/tests/sys/shell'
/home/cbuec/RIOTstuff/riot-vanilla/RIOT/dist/tools/pyterm/pyterm -p "/dev/ttyACM0" -b "115200" -ln "/tmp/pyterm-cbuec" -rn "2025-11-18_00.40.42-tests_shell-seeedstudio-xiao-nrf52840-sense"
2025-11-18 00:40:42,597 # Connect to serial port /dev/ttyACM0
Welcome to pyterm!
Type '/exit' to exit.
help
2025-11-18 00:40:44,657 # help
2025-11-18 00:40:44,663 # Command              Description
2025-11-18 00:40:44,664 # ---------------------------------------
2025-11-18 00:40:44,666 # bufsize              Get the shell's buffer size
2025-11-18 00:40:44,669 # start_test           starts a test
2025-11-18 00:40:44,672 # end_test             ends a test
2025-11-18 00:40:44,676 # echo                 prints the input command
2025-11-18 00:40:44,680 # empty                print nothing on command
2025-11-18 00:40:44,684 # periodic             periodically print command
2025-11-18 00:40:44,687 # xfa_test1            xfa test command 1
2025-11-18 00:40:44,690 # xfa_test2            xfa test command 2
2025-11-18 00:40:44,695 # app_metadata         Returns application metadata
2025-11-18 00:40:44,699 # pm                   interact with layered PM subsystem
2025-11-18 00:40:44,704 # ps                   Prints information about running threads.
2025-11-18 00:40:44,708 # version              Prints current RIOT_VERSION
2025-11-18 00:40:44,711 # bootloader           Reboot to bootloader
2025-11-18 00:40:44,714 # reboot               Reboot the node
>

maribu · 2025-11-18T08:40:18Z

This cannot easily be backported, as the thread unsafe ring buffer did not make it into the release.

Maybe we should just leave this for next release?

Or should we port this to the old state of the CDC ACM driver?

mguetschow · 2025-11-18T08:44:51Z

sys/usb/usbus/cdc/acm/cdc_acm.c

+    if (cdcacm->state == USBUS_CDC_ACM_LINE_STATE_DISCONNECTED) {
+        return len;


Shouldn't this rather return 0, as no bytes were actually added to the ring buffer? See

RIOT/sys/include/usb/usbus/cdc/acm.h

Lines 194 to 204 in bcd09d3

/**

* @brief Submit bytes to the CDC ACM handler

*

* @param[in] cdcacm USBUS CDC ACM handler context

* @param[in] buf buffer to submit

* @param[in] len length of the submitted buffer

*

* @return Number of bytes added to the CDC ACM ring buffer

*/

size_t usbus_cdc_acm_submit(usbus_cdcacm_device_t *cdcacm,

const uint8_t *buf, size_t len);

That would cause problems with stdio, as stdio_write() is expected to either return the number of bytes written in absence of an error, or an error if writing bytes won't work.

If it would return 0, the stdio implementation on top would just keep calling stdio_write() in a loop until all data has been written - which would be an endless loop here.

We should either change the doc to "number of bytes handled" with discarding being handled, or change the return type to ssize_t and return an error when discarding data.

Discarding the data here and not reporting an error doesn't really change the API contract, btw. Even if we would go for ssize_t, getting a success return code is no guarantee that the data will be transferred to the Linux host: They may have been successfully added to the ringbuffer and subsequently a disconnect may happen before the data left the ringbuffer.

Thanks for the explanation. I think I'd favor a telling error code over a silent drop. "bytes handled" does not really tell the user much.

sys/usb/cdc_acm: fix garbage input when disconnected

182c7ec

benpicco requested review from aabadie, bergzand and dylad as code owners November 17, 2025 20:09

github-actions bot added Area: USB Area: Universal Serial Bus Area: sys Area: System labels Nov 17, 2025

benpicco requested review from kfessel and maribu November 17, 2025 20:10

sys/usb_cdc_acm_stdio: use isrpipe_write()

494e171

benpicco added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Nov 17, 2025

maribu added the Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch label Nov 17, 2025

maribu enabled auto-merge November 17, 2025 20:24

maribu approved these changes Nov 17, 2025

View reviewed changes

maribu added this pull request to the merge queue Nov 17, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 18, 2025

crasbe added this pull request to the merge queue Nov 18, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 18, 2025

crasbe added this pull request to the merge queue Nov 18, 2025

Merged via the queue into RIOT-OS:master with commit bcd09d3 Nov 18, 2025
29 checks passed

mguetschow reviewed Nov 18, 2025

View reviewed changes

benpicco deleted the usbus/cdc/acm-fix_garbage branch November 18, 2025 10:44

maribu mentioned this pull request Nov 18, 2025

sys/usb/cdc_acm: Change API to report errors #21894

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

sys/usb/cdc_acm: fix garbage input when disconnected #21890

sys/usb/cdc_acm: fix garbage input when disconnected #21890

benpicco commented Nov 17, 2025 •

edited by crasbe

Loading

Uh oh!

riot-ci commented Nov 17, 2025

Uh oh!

maribu commented Nov 17, 2025

Uh oh!

crasbe commented Nov 17, 2025

Uh oh!

crasbe commented Nov 17, 2025

Uh oh!

crasbe commented Nov 17, 2025

Uh oh!

crasbe commented Nov 17, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maribu commented Nov 18, 2025

Uh oh!

mguetschow Nov 18, 2025

Uh oh!

maribu Nov 18, 2025

Uh oh!

mguetschow Nov 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		if (cdcacm->state == USBUS_CDC_ACM_LINE_STATE_DISCONNECTED) {
		return len;

	/**
	* @brief Submit bytes to the CDC ACM handler
	*
	* @param[in] cdcacm USBUS CDC ACM handler context
	* @param[in] buf buffer to submit
	* @param[in] len length of the submitted buffer
	*
	* @return Number of bytes added to the CDC ACM ring buffer
	*/
	size_t usbus_cdc_acm_submit(usbus_cdcacm_device_t *cdcacm,
	const uint8_t *buf, size_t len);

sys/usb/cdc_acm: fix garbage input when disconnected #21890

sys/usb/cdc_acm: fix garbage input when disconnected #21890

Conversation

benpicco commented Nov 17, 2025 • edited by crasbe Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution description

Testing procedure

master

with this patch

Issues/PRs references

Uh oh!

riot-ci commented Nov 17, 2025

Murdock results

Artifacts

Uh oh!

maribu commented Nov 17, 2025

Uh oh!

crasbe commented Nov 17, 2025

Uh oh!

crasbe commented Nov 17, 2025

Uh oh!

crasbe commented Nov 17, 2025

Uh oh!

crasbe commented Nov 17, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maribu commented Nov 18, 2025

Uh oh!

mguetschow Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

maribu Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

mguetschow Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

benpicco commented Nov 17, 2025 •

edited by crasbe

Loading