diff mbox series

[RFC] usb_wwan : add locking around shared port data in two FIXME-marked places

Message ID 20250620101747.39037-1-abinashsinghlalotra@gmail.com
State New
Headers show
Series [RFC] usb_wwan : add locking around shared port data in two FIXME-marked places | expand

Commit Message

Abinash Singh June 20, 2025, 10:17 a.m. UTC
Fix two locking-related FIXME comments by adding a mutex
 to protect shared fields in `usb_wwan_port_private`.

- In `usb_wwan_dtr_rts()`, access to `rts_state`
and `dtr_state` is now protected by `portdata->lock`.
- In `usb_wwan_tiocmset()`, access to `rts_state`
 and `dtr_state` is now also synchronized with the same mutex.

These changes prevent possible data races
and inconsistent state updates when the port is written concurrently.

Signed-off-by: Abinash Singh <abinashsinghlalotra@gmail.com>

---
usb_wwan_chars_in_buffer() have this:
/* FIXME: This locking is insufficient as this_urb may
		   go unused during the test */
How can we do proper locking there ?
Do we need to lock portdata in other places also ? I see no other FIXME related to locking

Thank You

Regards

Abinash
---
 drivers/usb/serial/usb-wwan.h | 3 +++
 drivers/usb/serial/usb_wwan.c | 7 ++++---
 2 files changed, 7 insertions(+), 3 deletions(-)
diff mbox series

Patch

diff --git a/drivers/usb/serial/usb-wwan.h b/drivers/usb/serial/usb-wwan.h
index 519101945..3cc0d4ef4 100644
--- a/drivers/usb/serial/usb-wwan.h
+++ b/drivers/usb/serial/usb-wwan.h
@@ -59,6 +59,9 @@  struct usb_wwan_port_private {
 	int ri_state;
 
 	unsigned long tx_start_time[N_OUT_URB];
+
+	/* Locking */
+	struct mutex lock;
 };
 
 #endif /* __LINUX_USB_USB_WWAN */
diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c
index 0017f6e96..4bd81b21f 100644
--- a/drivers/usb/serial/usb_wwan.c
+++ b/drivers/usb/serial/usb_wwan.c
@@ -80,10 +80,10 @@  void usb_wwan_dtr_rts(struct usb_serial_port *port, int on)
 		return;
 
 	portdata = usb_get_serial_port_data(port);
-	/* FIXME: locking */
+	mutex_lock(&portdata->lock);
 	portdata->rts_state = on;
 	portdata->dtr_state = on;
-
+	mutex_unlock(&portdata->lock);
 	usb_wwan_send_setup(port);
 }
 EXPORT_SYMBOL(usb_wwan_dtr_rts);
@@ -120,7 +120,7 @@  int usb_wwan_tiocmset(struct tty_struct *tty,
 	if (!intfdata->use_send_setup)
 		return -EINVAL;
 
-	/* FIXME: what locks portdata fields ? */
+	mutex_lock(&portdata->lock);
 	if (set & TIOCM_RTS)
 		portdata->rts_state = 1;
 	if (set & TIOCM_DTR)
@@ -130,6 +130,7 @@  int usb_wwan_tiocmset(struct tty_struct *tty,
 		portdata->rts_state = 0;
 	if (clear & TIOCM_DTR)
 		portdata->dtr_state = 0;
+	mutex_unlock(&portdata->lock);
 	return usb_wwan_send_setup(port);
 }
 EXPORT_SYMBOL(usb_wwan_tiocmset);