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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250620101747.39037-1-abinashsinghlalotra@gmail.com>
Date: Fri, 20 Jun 2025 15:47:47 +0530
From: Abinash Singh <abinashlalotra@...il.com>
To: johan@...nel.org
Cc: gregkh@...uxfoundation.org,
	linux-usb@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Abinash Singh <abinashsinghlalotra@...il.com>
Subject: [RFC PATCH] usb_wwan : add locking around shared port data in two FIXME-marked places

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@...il.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 --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);
-- 
2.43.0