USB: cdc-acm: fix break reporting
Subsystems (3)
cover USB: cdc-acm: fix break reporting
This fixes a couple of bugs in the recently added support for reporting break events.
Note that the offending commit was backported to stable.
Johan
Johan Hovold (2):
USB: cdc-acm: fix racy tty buffer accesses
USB: cdc-acm: fix break detection
drivers/usb/class/cdc-acm.c | 8 ++++++++ 1 file changed, 8 insertions(+)
Signature · 1 line
2.32.0
PATCH 1/2 USB: cdc-acm: fix racy tty buffer accesses
A recent change that started reporting break events to the line discipline caused the tty-buffer insertions to no longer be serialised by inserting events also from the completion handler for the interrupt endpoint.
Completion calls for distinct endpoints are not guaranteed to be serialised. For example, in case a host-controller driver uses bottom-half completion, the interrupt and bulk-in completion handlers can end up running in parallel on two CPUs (high-and low-prio tasklets, respectively) thereby breaking the tty layer's single producer assumption.
Fix this by holding the read lock also when inserting characters from the bulk endpoint.
--- drivers/usb/class/cdc-acm.c | 5 +++++ 1 file changed, 5 insertions(+)
drivers/usb/class/cdc-acm.c +5 -0
@@ -475,11 +475,16 @@ static int acm_submit_read_urbs(struct acm *acm, gfp_t mem_flags)static void acm_process_read_urb (struct acm *acm,struct urb *urb) { +unsigned long flags; +if (!urb->actual_length)return ; +spin_lock_irqsave (&acm->read_lock, flags);tty_insert_flip_string (&acm->port, urb->transfer_buffer, urb->actual_length); +spin_unlock_irqrestore (&acm->read_lock, flags); +tty_flip_buffer_push (&acm->port); }
Signature · 1 line
2.32.0
Re: [PATCH 1/2] USB: cdc-acm: fix racy tty buffer accesses
Quoted 12 lines · Johan Hovold
A recent change that started reporting break events to the line discipline caused the tty-buffer insertions to no longer be serialised by inserting events also from the completion handler for the interrupt endpoint.
Completion calls for distinct endpoints are not guaranteed to be serialised. For example, in case a host-controller driver uses bottom-half completion, the interrupt and bulk-in completion handlers can end up running in parallel on two CPUs (high-and low-prio tasklets, respectively) thereby breaking the tty layer's single producer assumption.
Quoted 6 lines
Fix this by holding the read lock also when inserting characters from the bulk endpoint.
Fixes 08dff274edda ("cdc-acm: fix BREAK rx code path adding necessary calls")Signed-off-by Johan Hovold <[email protected]>
PATCH 2/2 USB: cdc-acm: fix break reporting
In reply to Johan HovoldA recent change that started reporting break events forgot to push the event to the line discipline, which meant that a detected break would not be reported until further characters had been receive (the port could even have been closed and reopened in between).
--- drivers/usb/class/cdc-acm.c | 3 +++ 1 file changed, 3 insertions(+)
drivers/usb/class/cdc-acm.c +3 -0
@@ -340,6 +340,9 @@ static void acm_process_notification(struct acm *acm, unsigned char *buf) acm->iocount.overrun++;spin_unlock_irqrestore (&acm->read_lock, flags); +if (newctrl &ACM_CTRL_BRK ) +tty_flip_buffer_push (&acm->port); +if (difference)wake_up_all (&acm->wioctl);
Signature · 1 line
2.32.0
Quoted 8 lines · Johan Hovold
A recent change that started reporting break events forgot to push the event to the line discipline, which meant that a detected break would not be reported until further characters had been receive (the port could even have been closed and reopened in between).
Fixes 08dff274edda ("cdc-acm: fix BREAK rx code path adding necessary calls")Signed-off-by Johan Hovold <[email protected]>