* [PATCH] i2c: rust: fix I2cAdapter refcounts double increment
@ 2026-05-24 18:11 Nicolás Antinori
2026-05-25 2:56 ` Onur Özkan
0 siblings, 1 reply; 3+ messages in thread
From: Nicolás Antinori @ 2026-05-24 18:11 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,
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 call to
`bindings::i2c_get_adapter()` inside `I2cAdapter::inc_ref()`.
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.
Signed-off-by: Nicolás Antinori <nico.antinori.7@gmail.com>
---
rust/kernel/i2c.rs | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs
index 7b908f0c5a58..ca6476cf4399 100644
--- a/rust/kernel/i2c.rs
+++ b/rust/kernel/i2c.rs
@@ -405,7 +405,11 @@ 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() })
+ Ok(unsafe {
+ ARef::from_raw(NonNull::new_unchecked(
+ adapter.as_ptr().cast::<I2cAdapter<device::Normal>>(),
+ ))
+ })
}
}
--
2.53.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] i2c: rust: fix I2cAdapter refcounts double increment
2026-05-24 18:11 [PATCH] i2c: rust: fix I2cAdapter refcounts double increment Nicolás Antinori
@ 2026-05-25 2:56 ` Onur Özkan
2026-05-26 16:48 ` Nicolás Antinori
0 siblings, 1 reply; 3+ messages in thread
From: Onur Özkan @ 2026-05-25 2:56 UTC (permalink / raw)
To: Nicolás Antinori
Cc: Igor Korotin, Alice Ryhl, Andreas Hindborg, Benno Lossin,
Björn Roy Baron, Boqun Feng, Daniel Almeida,
Danilo Krummrich, Gary Guo, Miguel Ojeda, Trevor Gross,
linux-kernel, rust-for-linux
Hi Nicolás
On Sun, 24 May 2026 15:11:50 -0300
Nicolás Antinori <nico.antinori.7@gmail.com> 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 call to
> `bindings::i2c_get_adapter()` inside `I2cAdapter::inc_ref()`.
>
> 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.
Including the link tag would be great.
>
> Signed-off-by: Nicolás Antinori <nico.antinori.7@gmail.com>
> ---
> rust/kernel/i2c.rs | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs
> index 7b908f0c5a58..ca6476cf4399 100644
> --- a/rust/kernel/i2c.rs
> +++ b/rust/kernel/i2c.rs
> @@ -405,7 +405,11 @@ 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() })
> + Ok(unsafe {
> + ARef::from_raw(NonNull::new_unchecked(
> + adapter.as_ptr().cast::<I2cAdapter<device::Normal>>(),
> + ))
> + })
> }
> }
>
> --
> 2.53.0
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] i2c: rust: fix I2cAdapter refcounts double increment
2026-05-25 2:56 ` Onur Özkan
@ 2026-05-26 16:48 ` Nicolás Antinori
0 siblings, 0 replies; 3+ messages in thread
From: Nicolás Antinori @ 2026-05-26 16:48 UTC (permalink / raw)
To: Onur Özkan
Cc: Igor Korotin, Alice Ryhl, Andreas Hindborg, Benno Lossin,
Björn Roy Baron, Boqun Feng, Daniel Almeida,
Danilo Krummrich, Gary Guo, Miguel Ojeda, Trevor Gross,
linux-kernel, rust-for-linux
Hello Onur
On Mon, 25 May 2026 05:56:12 +0300
Onur Özkan <work@onurozkan.dev> wrote:
> Hi Nicolás
>
> On Sun, 24 May 2026 15:11:50 -0300
> Nicolás Antinori <nico.antinori.7@gmail.com> 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 call to `bindings::i2c_get_adapter()` inside
> > `I2cAdapter::inc_ref()`.
> >
> > 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.
>
> Including the link tag would be great.
I will include the missing Link tag in a v2. Apologies for the
oversight.
Separately, Sashiko reported a critical issue in reply to this patch:
https://sashiko.dev/#/patchset/20260524181151.24988-1-nico.antinori.7@gmail.com?part=1
Since the report indicates this is a pre-existing issue, I believe it
is better to address it in a separate patch. I will send a v2 for this
change shortly.
Thank you
>
> >
> > Signed-off-by: Nicolás Antinori <nico.antinori.7@gmail.com>
> > ---
> > rust/kernel/i2c.rs | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs
> > index 7b908f0c5a58..ca6476cf4399 100644
> > --- a/rust/kernel/i2c.rs
> > +++ b/rust/kernel/i2c.rs
> > @@ -405,7 +405,11 @@ 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() })
> > + Ok(unsafe {
> > + ARef::from_raw(NonNull::new_unchecked(
> > +
> > adapter.as_ptr().cast::<I2cAdapter<device::Normal>>(),
> > + ))
> > + })
> > }
> > }
> >
> > --
> > 2.53.0
> >
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-05-26 16:48 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-24 18:11 [PATCH] i2c: rust: fix I2cAdapter refcounts double increment Nicolás Antinori
2026-05-25 2:56 ` Onur Özkan
2026-05-26 16:48 ` Nicolás Antinori
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.