From 45311bd6f9bad7f095b0663dfb1fcbffd1debd0f Mon Sep 17 00:00:00 2001
From: PulkoMandy <pulkomandy@pulkomandy.tk>
Date: Sat, 22 Jul 2023 19:47:48 +0200
Subject: [PATCH] usb_rndis: synchronize writes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The write function can be called concurrently by multiple threads. The
way it is implemented now means this desn't work, since there is no
guarantee the correct thread will be released from the semaphore by the
USB completion callback.

I tried to allow mutiple requests to run "in parallel" (really letting
the USB stack schedule them) by having he callback track which thread to
wake up (using send_message/receive_message as a synchronization tool)
but that still resulted in lockups.

The simplest solution is to ensure there is only a single thread doing a
write transaction at a time, which is achieved here with an extra mutex.

Fixes #18521.

Change-Id: I0b737acab6f5665cbe5b0e40a20ce99c16bdf21c
Reviewed-on: https://review.haiku-os.org/c/haiku/+/6707
Reviewed-by: Jérôme Duval <jerome.duval@gmail.com>
---
 .../network/ether/usb_rndis/RNDISDevice.cpp   | 46 ++++++++++++++++++-
 .../network/ether/usb_rndis/RNDISDevice.h     |  1 +
 2 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/src/add-ons/kernel/drivers/network/ether/usb_rndis/RNDISDevice.cpp b/src/add-ons/kernel/drivers/network/ether/usb_rndis/RNDISDevice.cpp
index 166d2c4062..b132b28d2b 100644
--- a/src/add-ons/kernel/drivers/network/ether/usb_rndis/RNDISDevice.cpp
+++ b/src/add-ons/kernel/drivers/network/ether/usb_rndis/RNDISDevice.cpp
@@ -73,6 +73,7 @@ RNDISDevice::RNDISDevice(usb_device device)
 		fWriteEndpoint(0),
 		fNotifyReadSem(-1),
 		fNotifyWriteSem(-1),
+		fLockWriteSem(-1),
 		fNotifyControlSem(-1),
 		fReadHeader(NULL),
 		fLinkStateChangeSem(-1),
@@ -102,6 +103,12 @@ RNDISDevice::RNDISDevice(usb_device device)
 		return;
 	}
 
+	fLockWriteSem = create_sem(1, DRIVER_NAME"_lock_write");
+	if (fNotifyWriteSem < B_OK) {
+		TRACE_ALWAYS("failed to create write lock sem\n");
+		return;
+	}
+
 	fNotifyControlSem = create_sem(0, DRIVER_NAME"_notify_control");
 	if (fNotifyControlSem < B_OK) {
 		TRACE_ALWAYS("failed to create control notify sem\n");
@@ -123,6 +130,10 @@ RNDISDevice::~RNDISDevice()
 		delete_sem(fNotifyReadSem);
 	if (fNotifyWriteSem >= B_OK)
 		delete_sem(fNotifyWriteSem);
+	if (fLockWriteSem >= B_OK)
+		delete_sem(fLockWriteSem);
+	if (fNotifyControlSem >= B_OK)
+		delete_sem(fNotifyControlSem);
 
 	if (!fRemoved)
 		gUSBModule->cancel_queued_transfers(fNotifyEndpoint);
@@ -326,6 +337,26 @@ RNDISDevice::Read(uint8 *buffer, size_t *numBytes)
 }
 
 
+class SemLocker {
+public:
+	SemLocker(sem_id sem)
+		: fSem(sem)
+	{
+		fStatus = acquire_sem(fSem);
+	}
+
+	~SemLocker()
+	{
+		if (fStatus == B_OK)
+			release_sem(fSem);
+	}
+
+	status_t fStatus;
+private:
+	sem_id fSem;
+};
+
+
 status_t
 RNDISDevice::Write(const uint8 *buffer, size_t *numBytes)
 {
@@ -348,13 +379,23 @@ RNDISDevice::Write(const uint8 *buffer, size_t *numBytes)
 	vec[1].iov_base = (void*)buffer;
 	vec[1].iov_len = *numBytes;
 
-	status_t result = gUSBModule->queue_bulk_v(fWriteEndpoint, vec, 2, _WriteCallback, this);
+	SemLocker mutex(fLockWriteSem);
+	status_t result = mutex.fStatus;
+	if (result < B_OK) {
+		*numBytes = 0;
+		return result;
+	}
+
+	result = gUSBModule->queue_bulk_v(fWriteEndpoint, vec, 2, _WriteCallback, this);
 	if (result != B_OK) {
 		*numBytes = 0;
 		return result;
 	}
 
-	result = acquire_sem_etc(fNotifyWriteSem, 1, B_CAN_INTERRUPT, 0);
+	do {
+		result = acquire_sem_etc(fNotifyWriteSem, 1, B_CAN_INTERRUPT, 0);
+	} while (result == B_INTERRUPTED);
+
 	if (result < B_OK) {
 		*numBytes = 0;
 		return result;
@@ -372,6 +413,7 @@ RNDISDevice::Write(const uint8 *buffer, size_t *numBytes)
 	}
 
 	*numBytes = fActualLengthWrite;
+
 	return B_OK;
 }
 
diff --git a/src/add-ons/kernel/drivers/network/ether/usb_rndis/RNDISDevice.h b/src/add-ons/kernel/drivers/network/ether/usb_rndis/RNDISDevice.h
index 60a23655a1..872739faa5 100644
--- a/src/add-ons/kernel/drivers/network/ether/usb_rndis/RNDISDevice.h
+++ b/src/add-ons/kernel/drivers/network/ether/usb_rndis/RNDISDevice.h
@@ -76,6 +76,7 @@ static	void				_NotifyCallback(void *cookie, int32 status,
 		int32				fStatusWrite;
 		sem_id				fNotifyReadSem;
 		sem_id				fNotifyWriteSem;
+		sem_id				fLockWriteSem;
 		sem_id				fNotifyControlSem;
 
 		uint8				fNotifyBuffer[8];