[PATCH v2] i2c: rust: avoid locking when performing I2cAdapter::inc_ref

[Nicolás Antinori]

The current implementation of `<I2cAdapter as
AlwaysRefCounted>::inc_ref` relies on the C function `i2c_get_adapter`
to increment module and device counters. This function acquires a lock,
looks for the adapter in the IDR table, and, if found, increments the
named counters before returning the adapter.

In the Rust API, the `I2cAdapter::get` method returns an
`ARef<I2cAdapter>` upon success. Incrementing this reference count in an
atomic context (for example, via `ARef::clone`, which relies on
`AlwaysRefCounted::inc_ref`) could trigger a sleep-in-atomic bug due to
the mutex locking inside `i2c_get_adapter`.

Since cloning an `ARef` implies we already hold a valid reference to the
adapter, the IDR table lookup and its associated lock are unnecessary.

The fix consists of bypassing `i2c_get_adapter` and instead calling
`__i2c_adapter_get` directly to increment the counters.

Reported-by: Sashiko <sashiko-bot@xxxxxxxxxx>
Closes: https://sashiko.dev/#/patchset/20260524181151.24988-1-nico.antinori.7@xxxxxxxxx
Signed-off-by: Nicolás Antinori <nico.antinori.7@xxxxxxxxx>
---
Note: There's Trevor Chan's patch [2] (make `AlwaysRefCounted::inc_ref`
associated function) in review. If this patch lands first it may cause
conflicts with that one. Please let me know if it is better to make this
patch dependant on Trevor's one or wait until it merges.

Changelog:
- v2:
  - Implemented suggestion by Gary and Igor to add a C API and call it
    directly from Rust.
- v1: [1]

v1 Note:

Citing the second part of Sashiko's report:

> Furthermore, if the adapter is unregistered and removed from the IDR,
> bindings::i2c_get_adapter() will return NULL and fail to increment the
> reference count. Since inc_ref() ignores the return value, wouldn't
> dropping that cloned ARef unconditionally call dec_ref()
> (i2c_put_adapter)?
>
> Could this lead to an underflow, double-put, and a use-after-free of
> the adapter and its module? Or if the IDR index was reused, might it
> increment the new adapter's refcount while decrementing the old one
> twice?

I don't believe this situation is possible. When `i2c_del_adapter` is
executed in `i2c-core-base.c`, the kernel waits for all references to be
dropped prior to removing the device from the IDR. This guarantees that
no `ARef` is still alive when the IDR removal happens, effectively
eliminating the risk of an underflow, double-put, or calling `dec_ref`
on an invalid reference.

[1] https://lore.kernel.org/all/20260615201141.8920-1-nico.antinori.7@xxxxxxxxx/
[2] https://lore.kernel.org/rust-for-linux/20260628100731.64885-1-trev@xxxxxxxxxxxx/T/#u

 drivers/i2c/i2c-core-base.c | 17 +++++++++++------
 include/linux/i2c.h         |  1 +
 rust/kernel/i2c.rs          |  4 +++-
 3 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index ddaacf876dad..03ad1bb539e9 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -2632,21 +2632,26 @@ i2c_new_scanned_device(struct i2c_adapter *adap,
 }
 EXPORT_SYMBOL_GPL(i2c_new_scanned_device);

+bool __i2c_adapter_get(struct i2c_adapter *adapter)
+{
+	if (try_module_get(adapter->owner)) {
+		get_device(&adapter->dev);
+		return true;
+	}
+	return false;
+}
+EXPORT_SYMBOL(__i2c_adapter_get);
+
 struct i2c_adapter *i2c_get_adapter(int nr)
 {
 	struct i2c_adapter *adapter;

 	mutex_lock(&core_lock);
 	adapter = idr_find(&i2c_adapter_idr, nr);
-	if (!adapter)
-		goto exit;

-	if (try_module_get(adapter->owner))
-		get_device(&adapter->dev);
-	else
+	if (adapter && !__i2c_adapter_get(adapter))
 		adapter = NULL;

- exit:
 	mutex_unlock(&core_lock);
 	return adapter;
 }
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 14ab4d3055af..56832c8e58f5 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -907,6 +907,7 @@ static inline bool i2c_client_has_driver(struct i2c_client *client)
 void i2c_clients_command(struct i2c_adapter *adap,
 			 unsigned int cmd, void *arg);

+bool __i2c_adapter_get(struct i2c_adapter *adapter);
 struct i2c_adapter *i2c_get_adapter(int nr);
 void i2c_put_adapter(struct i2c_adapter *adap);
 unsigned int i2c_adapter_depth(struct i2c_adapter *adapter);
diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs
index 0487bae811fb..2fc818da80cf 100644
--- a/rust/kernel/i2c.rs
+++ b/rust/kernel/i2c.rs
@@ -419,7 +419,9 @@ unsafe impl AlwaysRefCounted for I2cAdapter {
     #[inline]
     fn inc_ref(&self) {
         // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
-        unsafe { bindings::i2c_get_adapter(self.index()) };
+        unsafe {
+            bindings::__i2c_adapter_get(&raw mut (*self.as_raw()));
+        }
     }

     #[inline]
--
2.47.3