Linux I2C development
 help / color / mirror / Atom feed
* [PATCH] i2c: core: move idr_remove() in i2c_del_adapter() before device_unregister()
@ 2026-06-18 18:46 Tejas Mutalikdesai
  2026-06-18 20:52 ` Wolfram Sang
  0 siblings, 1 reply; 4+ messages in thread
From: Tejas Mutalikdesai @ 2026-06-18 18:46 UTC (permalink / raw)
  To: andi.shyti
  Cc: wsa, vladimir_zapolskiy, linux-i2c, linux-kernel,
	Tejas Mutalikdesai, syzbot+c0291c8c9aaa473c7721

There is a race between i2c_del_adapter() and i2c_get_adapter() that
can trigger a "refcount_t: addition on 0; use-after-free" warning.

The sequence is:

  1. i2c_del_adapter() calls device_unregister(), which drops the
     device refcount to zero (the adapter's release callback fires
     and signals dev_released).

  2. The adapter is still in i2c_adapter_idr because idr_remove()
     hasn't been called yet.

  3. A concurrent i2c_get_adapter() calls idr_find() under core_lock
     and finds the adapter.  try_module_get() succeeds because the
     owning module is still MODULE_STATE_LIVE (this is a hot-remove
     path, not a module unload).

  4. get_device() is called on a kobject whose refcount is already
     zero, triggering refcount_warn_saturate() with REFCOUNT_ADD_UAF.

Fixing this by moving the idr_remove() call to before device_unregister().
Once the adapter is removed from the IDR, any concurrent
i2c_get_adapter() will get NULL from idr_find() and return -ENODEV.
Callers that already hold a device reference are unaffected:
wait_for_completion() correctly waits for them to release it via
i2c_put_adapter().

REPRODUCTION AND VALIDATION: Tested by inserting a msleep(500) after wait_for_completion() to widen
the race window, and using a kernel module that spawns two kthreads: one
calling i2c_del_adapter() and another calling i2c_get_adapter()
concurrently while the module remains MODULE_STATE_LIVE.  Without the
fix, the WARNING fires reliably.  With the fix, i2c_get_adapter()
returns NULL and no WARNING is observed.

Reported-by: syzbot+c0291c8c9aaa473c7721@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=c0291c8c9aaa473c7721
Fixes: 611e12ea0f12 ("i2c: core: manage i2c bus device refcount in i2c_[get|put]_adapter")
Signed-off-by: Tejas Mutalikdesai <tejasmutalikdesai@gmail.com>
---
 drivers/i2c/i2c-core-base.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index a2132d70fb36..780ce42fa950 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -1801,6 +1801,16 @@ void i2c_del_adapter(struct i2c_adapter *adap)
 
 	debugfs_remove_recursive(adap->debugfs);
 
+	/* Remove from IDR first so that a concurrent i2c_get_adapter() can
+	 * no longer find this adapter.  Without this ordering,
+	 * i2c_get_adapter() can find the adapter via idr_find() and call
+	 * get_device() after device_unregister() has already dropped the
+	 * refcount to zero, triggering a use-after-free warning.
+	 */
+	mutex_lock(&core_lock);
+	idr_remove(&i2c_adapter_idr, adap->nr);
+	mutex_unlock(&core_lock);
+
 	/* wait until all references to the device are gone
 	 *
 	 * FIXME: This is old code and should ideally be replaced by an
@@ -1812,11 +1822,6 @@ void i2c_del_adapter(struct i2c_adapter *adap)
 	device_unregister(&adap->dev);
 	wait_for_completion(&adap->dev_released);
 
-	/* free bus id */
-	mutex_lock(&core_lock);
-	idr_remove(&i2c_adapter_idr, adap->nr);
-	mutex_unlock(&core_lock);
-
 	/* Clear the device structure in case this adapter is ever going to be
 	   added again */
 	memset(&adap->dev, 0, sizeof(adap->dev));
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] i2c: core: move idr_remove() in i2c_del_adapter() before device_unregister()
  2026-06-18 18:46 [PATCH] i2c: core: move idr_remove() in i2c_del_adapter() before device_unregister() Tejas Mutalikdesai
@ 2026-06-18 20:52 ` Wolfram Sang
  2026-06-19  3:22   ` Tejas MD
  0 siblings, 1 reply; 4+ messages in thread
From: Wolfram Sang @ 2026-06-18 20:52 UTC (permalink / raw)
  To: Tejas Mutalikdesai
  Cc: andi.shyti, wsa, vladimir_zapolskiy, linux-i2c, linux-kernel,
	syzbot+c0291c8c9aaa473c7721

[-- Attachment #1: Type: text/plain, Size: 2162 bytes --]

On Fri, Jun 19, 2026 at 12:16:10AM +0530, Tejas Mutalikdesai wrote:
> There is a race between i2c_del_adapter() and i2c_get_adapter() that
> can trigger a "refcount_t: addition on 0; use-after-free" warning.
> 
> The sequence is:
> 
>   1. i2c_del_adapter() calls device_unregister(), which drops the
>      device refcount to zero (the adapter's release callback fires
>      and signals dev_released).
> 
>   2. The adapter is still in i2c_adapter_idr because idr_remove()
>      hasn't been called yet.
> 
>   3. A concurrent i2c_get_adapter() calls idr_find() under core_lock
>      and finds the adapter.  try_module_get() succeeds because the
>      owning module is still MODULE_STATE_LIVE (this is a hot-remove
>      path, not a module unload).
> 
>   4. get_device() is called on a kobject whose refcount is already
>      zero, triggering refcount_warn_saturate() with REFCOUNT_ADD_UAF.
> 
> Fixing this by moving the idr_remove() call to before device_unregister().
> Once the adapter is removed from the IDR, any concurrent
> i2c_get_adapter() will get NULL from idr_find() and return -ENODEV.
> Callers that already hold a device reference are unaffected:
> wait_for_completion() correctly waits for them to release it via
> i2c_put_adapter().
> 
> REPRODUCTION AND VALIDATION: Tested by inserting a msleep(500) after wait_for_completion() to widen
> the race window, and using a kernel module that spawns two kthreads: one
> calling i2c_del_adapter() and another calling i2c_get_adapter()
> concurrently while the module remains MODULE_STATE_LIVE.  Without the
> fix, the WARNING fires reliably.  With the fix, i2c_get_adapter()
> returns NULL and no WARNING is observed.
> 
> Reported-by: syzbot+c0291c8c9aaa473c7721@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=c0291c8c9aaa473c7721
> Fixes: 611e12ea0f12 ("i2c: core: manage i2c bus device refcount in i2c_[get|put]_adapter")
> Signed-off-by: Tejas Mutalikdesai <tejasmutalikdesai@gmail.com>

I'd think this is fixed with b1a58ed9eab1 ("i2c: core: fix adapter
deregistration race") which went upstream this merge window.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] i2c: core: move idr_remove() in i2c_del_adapter() before device_unregister()
  2026-06-18 20:52 ` Wolfram Sang
@ 2026-06-19  3:22   ` Tejas MD
  2026-06-19  4:19     ` Wolfram Sang
  0 siblings, 1 reply; 4+ messages in thread
From: Tejas MD @ 2026-06-19  3:22 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: andi.shyti, wsa, vladimir_zapolskiy, linux-i2c, linux-kernel,
	syzbot+c0291c8c9aaa473c7721

Thanks for letting me know. Will check before raising the next patch.

Learnt something new :)


On Fri, Jun 19, 2026 at 2:22 AM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
>
> On Fri, Jun 19, 2026 at 12:16:10AM +0530, Tejas Mutalikdesai wrote:
> > There is a race between i2c_del_adapter() and i2c_get_adapter() that
> > can trigger a "refcount_t: addition on 0; use-after-free" warning.
> >
> > The sequence is:
> >
> >   1. i2c_del_adapter() calls device_unregister(), which drops the
> >      device refcount to zero (the adapter's release callback fires
> >      and signals dev_released).
> >
> >   2. The adapter is still in i2c_adapter_idr because idr_remove()
> >      hasn't been called yet.
> >
> >   3. A concurrent i2c_get_adapter() calls idr_find() under core_lock
> >      and finds the adapter.  try_module_get() succeeds because the
> >      owning module is still MODULE_STATE_LIVE (this is a hot-remove
> >      path, not a module unload).
> >
> >   4. get_device() is called on a kobject whose refcount is already
> >      zero, triggering refcount_warn_saturate() with REFCOUNT_ADD_UAF.
> >
> > Fixing this by moving the idr_remove() call to before device_unregister().
> > Once the adapter is removed from the IDR, any concurrent
> > i2c_get_adapter() will get NULL from idr_find() and return -ENODEV.
> > Callers that already hold a device reference are unaffected:
> > wait_for_completion() correctly waits for them to release it via
> > i2c_put_adapter().
> >
> > REPRODUCTION AND VALIDATION: Tested by inserting a msleep(500) after wait_for_completion() to widen
> > the race window, and using a kernel module that spawns two kthreads: one
> > calling i2c_del_adapter() and another calling i2c_get_adapter()
> > concurrently while the module remains MODULE_STATE_LIVE.  Without the
> > fix, the WARNING fires reliably.  With the fix, i2c_get_adapter()
> > returns NULL and no WARNING is observed.
> >
> > Reported-by: syzbot+c0291c8c9aaa473c7721@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=c0291c8c9aaa473c7721
> > Fixes: 611e12ea0f12 ("i2c: core: manage i2c bus device refcount in i2c_[get|put]_adapter")
> > Signed-off-by: Tejas Mutalikdesai <tejasmutalikdesai@gmail.com>
>
> I'd think this is fixed with b1a58ed9eab1 ("i2c: core: fix adapter
> deregistration race") which went upstream this merge window.
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] i2c: core: move idr_remove() in i2c_del_adapter() before device_unregister()
  2026-06-19  3:22   ` Tejas MD
@ 2026-06-19  4:19     ` Wolfram Sang
  0 siblings, 0 replies; 4+ messages in thread
From: Wolfram Sang @ 2026-06-19  4:19 UTC (permalink / raw)
  To: Tejas MD
  Cc: andi.shyti, wsa, vladimir_zapolskiy, linux-i2c, linux-kernel,
	syzbot+c0291c8c9aaa473c7721

[-- Attachment #1: Type: text/plain, Size: 288 bytes --]

Hi,

On Fri, Jun 19, 2026 at 08:52:47AM +0530, Tejas MD wrote:
> Thanks for letting me know. Will check before raising the next patch.
> 
> Learnt something new :)

:) Make sure you check linux-next, then. There might be something in  it
which is not in Linus' tree currently.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-06-19  4:20 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-18 18:46 [PATCH] i2c: core: move idr_remove() in i2c_del_adapter() before device_unregister() Tejas Mutalikdesai
2026-06-18 20:52 ` Wolfram Sang
2026-06-19  3:22   ` Tejas MD
2026-06-19  4:19     ` Wolfram Sang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox