* [PATCH v3] i2c: rust: fix I2cAdapter refcounts double increment
@ 2026-05-31 18:23 Nicolás Antinori
2026-06-06 13:38 ` Igor Korotin
0 siblings, 1 reply; 2+ messages in thread
From: Nicolás Antinori @ 2026-05-31 18:23 UTC (permalink / raw)
To: Igor Korotin
Cc: Nicolás Antinori, Alice Ryhl, Andreas Hindborg, Benno Lossin,
Björn Roy Baron, Boqun Feng, Daniel Almeida,
Danilo Krummrich, Gary Guo, Miguel Ojeda, Trevor Gross,
Onur Özkan, linux-kernel, rust-for-linux
When `I2cAdapter::get` executes, it first calls
`bindings::i2c_get_adapter()` which increments the device and module
reference counts. It then takes a reference to the raw pointer and
converts it to an `ARef` via `.into()`.
The implementation of `From<&T> for ARef<T>` where `T: AlwaysRefCounted`
unconditionally calls `T::inc_ref()`. This leads to a second increment
to the reference counts.
Since the returned `ARef` will only release a single reference when
dropped via `dec_ref()`, this leaks one device and module reference count
on every call.
This fix was suggested by sashiko.dev.
Link: https://sashiko.dev/#/patchset/20260521190937.248904-1-nico.antinori.7@gmail.com
Signed-off-by: Nicolás Antinori <nico.antinori.7@gmail.com>
---
v3:
- Remove unnecessary `.as_ptr()` and `NonNull` rewrap.
- Add SAFETY comment for `ARef::from_raw`.
- Small change in commit message to make it clearer.
v2:
- Add missing Link tag to commit message.
- https://lore.kernel.org/rust-for-linux/20260526194734.14378-1-nico.antinori.7@gmail.com
v1: https://lore.kernel.org/rust-for-linux/20260524181151.24988-1-nico.antinori.7@gmail.com
rust/kernel/i2c.rs | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs
index 7b908f0c5a58..c084a45b1916 100644
--- a/rust/kernel/i2c.rs
+++ b/rust/kernel/i2c.rs
@@ -405,7 +405,9 @@ pub fn get(index: i32) -> Result<ARef<Self>> {
// SAFETY: `adapter` is non-null and points to a live `i2c_adapter`.
// `I2cAdapter` is #[repr(transparent)], so this cast is valid.
- Ok(unsafe { (&*adapter.as_ptr().cast::<I2cAdapter<device::Normal>>()).into() })
+ // `i2c_get_adapter` returned the adapter with an incremented refcount, which we pass to
+ // the `ARef`.
+ Ok(unsafe { ARef::from_raw(adapter.cast::<I2cAdapter<device::Normal>>()) })
}
}
--
2.53.0
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH v3] i2c: rust: fix I2cAdapter refcounts double increment
2026-05-31 18:23 [PATCH v3] i2c: rust: fix I2cAdapter refcounts double increment Nicolás Antinori
@ 2026-06-06 13:38 ` Igor Korotin
0 siblings, 0 replies; 2+ messages in thread
From: Igor Korotin @ 2026-06-06 13:38 UTC (permalink / raw)
To: Nicolás Antinori
Cc: Alice Ryhl, Andreas Hindborg, Benno Lossin, Björn Roy Baron,
Boqun Feng, Daniel Almeida, Danilo Krummrich, Gary Guo,
Miguel Ojeda, Trevor Gross, Onur Özkan, linux-kernel,
rust-for-linux
Hello Nicolás
On 5/31/2026 7:23 PM, Nicolás Antinori wrote:
> When `I2cAdapter::get` executes, it first calls
> `bindings::i2c_get_adapter()` which increments the device and module
> reference counts. It then takes a reference to the raw pointer and
> converts it to an `ARef` via `.into()`.
>
> The implementation of `From<&T> for ARef<T>` where `T: AlwaysRefCounted`
> unconditionally calls `T::inc_ref()`. This leads to a second increment
> to the reference counts.
>
> Since the returned `ARef` will only release a single reference when
> dropped via `dec_ref()`, this leaks one device and module reference count
> on every call.
>
> This fix was suggested by sashiko.dev.
>
> Link: https://sashiko.dev/#/patchset/20260521190937.248904-1-nico.antinori.7@gmail.com
> Signed-off-by: Nicolás Antinori <nico.antinori.7@gmail.com>
> ---
> v3:
> - Remove unnecessary `.as_ptr()` and `NonNull` rewrap.
> - Add SAFETY comment for `ARef::from_raw`.
> - Small change in commit message to make it clearer.
> v2:
> - Add missing Link tag to commit message.
> - https://lore.kernel.org/rust-for-linux/20260526194734.14378-1-nico.antinori.7@gmail.com
> v1: https://lore.kernel.org/rust-for-linux/20260524181151.24988-1-nico.antinori.7@gmail.com
>
> rust/kernel/i2c.rs | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs
> index 7b908f0c5a58..c084a45b1916 100644
> --- a/rust/kernel/i2c.rs
> +++ b/rust/kernel/i2c.rs
> @@ -405,7 +405,9 @@ pub fn get(index: i32) -> Result<ARef<Self>> {
>
> // SAFETY: `adapter` is non-null and points to a live `i2c_adapter`.
> // `I2cAdapter` is #[repr(transparent)], so this cast is valid.
> - Ok(unsafe { (&*adapter.as_ptr().cast::<I2cAdapter<device::Normal>>()).into() })
> + // `i2c_get_adapter` returned the adapter with an incremented refcount, which we pass to
> + // the `ARef`.
> + Ok(unsafe { ARef::from_raw(adapter.cast::<I2cAdapter<device::Normal>>()) })
> }
> }
>
> --
> 2.53.0
>
Reviewed-by: Igor Korotin <igor.korotin@linux.dev>
Cheers
Igor
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-06-06 13:38 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-31 18:23 [PATCH v3] i2c: rust: fix I2cAdapter refcounts double increment Nicolás Antinori
2026-06-06 13:38 ` Igor Korotin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox