fix(i2c_spi): prevent use-after-free when stopping driver instances (#27723)

* fix(i2c_spi): prevent use-after-free when stopping driver instances

module_stop() deleted each instance before unlinking it from the global
i2c_spi_module_instances list. The instance *is* the intrusive list node,
so removeInstance()/List::remove() then dereferenced the freed node to fix
up the links. If another thread reused that heap block in the window, the
list got corrupted, sporadically leaving instances linked (a later start
reports "already running") or hard-faulting. This regressed in the 2020
array->linked-list refactor: the old array version only nulled an array
slot after delete, which was safe.

Unlink each instance before freeing it. Also only delete/unlink when the
task actually exited: if request_stop_and_wait() times out the work queue
may still reference the object, so leave it allocated and listed rather
than freeing it. module_stop() now returns -1 when an instance failed to
stop.

Signed-off-by: Jacob Dahl <dahl.jakejacob@gmail.com>

* Update platforms/common/i2c_spi_buses.cpp

* Update platforms/common/i2c_spi_buses.cpp

---------

Signed-off-by: Jacob Dahl <dahl.jakejacob@gmail.com>

platforms/common/i2c_spi_buses.cpp

@@ -733,14 +733,20 @@ int I2CSPIDriverBase::module_start(const BusCLIArguments &cli, BusInstanceIterat
 int I2CSPIDriverBase::module_stop(BusInstanceIterator &iterator)
 {
 	bool is_running = false;
+	bool stop_failed = false;
 
 	while (iterator.next()) {
 		if (iterator.instance()) {
-			I2CSPIDriverBase *instance = (I2CSPIDriverBase *)iterator.instance();
-			instance->request_stop_and_wait();
-			delete iterator.instance();
-			iterator.removeInstance();
 			is_running = true;
+			I2CSPIDriverBase *instance = (I2CSPIDriverBase *)iterator.instance();
+
+			if (instance->request_stop_and_wait()) {
+				iterator.removeInstance();
+				delete instance;
+
+			} else {
+				stop_failed = true;
+			}
 		}
 	}
 
@@ -749,7 +755,7 @@ int I2CSPIDriverBase::module_stop(BusInstanceIterator &iterator)
 		return -1;
 	}
 
-	return 0;
+	return stop_failed ? -1 : 0;
 }
 
 int I2CSPIDriverBase::module_status(BusInstanceIterator &iterator)
@@ -826,7 +832,7 @@ void I2CSPIDriverBase::print_status()
 #endif // CONFIG_SPI
 }
 
-void I2CSPIDriverBase::request_stop_and_wait()
+bool I2CSPIDriverBase::request_stop_and_wait()
 {
 	_task_should_exit.store(true);
 	ScheduleNow(); // wake up the task (in case it is not scheduled anymore or just to be faster)
@@ -837,7 +843,11 @@ void I2CSPIDriverBase::request_stop_and_wait()
 		// wait at most 2 sec
 	} while (++i < 100 && !_task_exited.load());
 
-	if (i >= 100) {
+	const bool exited = _task_exited.load();
+
+	if (!exited) {
 		PX4_ERR("Module did not respond to stop request");
 	}
+
+	return exited;
 }

platforms/common/include/px4_platform_common/i2c_spi_buses.h

@@ -314,7 +314,7 @@ protected:
 private:
 	static void custom_method_trampoline(void *argument);
 
-	void request_stop_and_wait();
+	bool request_stop_and_wait();
 
 	px4::atomic_bool _task_should_exit{false};
 	px4::atomic_bool _task_exited{false};