Subject: Re: [syzbot] [i2c?] WARNING: refcount bug in i2c_get_adapter (2) This email is created by automation to help kernel developers deal with a large volume of AI generated bug reports by decoding oopses into more actionable information. Decoded Backtrace 0. refcount_warn_saturate -- lib/refcount.c:25 (crash site) 11 #define REFCOUNT_WARN(str) WARN_ONCE(1, "refcount_t: " str ".\n") 12 13 void refcount_warn_saturate(refcount_t *r, 13 enum refcount_saturation_type t) 14 { 15 refcount_set(r, REFCOUNT_SATURATED); 16 17 switch (t) { 18 case REFCOUNT_ADD_NOT_ZERO_OVF: 19 case REFCOUNT_ADD_OVF: 20 REFCOUNT_WARN("saturated; leaking memory"); 21 break; 22 case REFCOUNT_ADD_UAF: 23 case REFCOUNT_SUB_UAF: -> 25 REFCOUNT_WARN("addition on 0; use-after-free"); 25 break; 26 case REFCOUNT_DEC_LEAK: 27 REFCOUNT_WARN("decrement hit 0; leaking memory"); 28 break; 29 default: 30 REFCOUNT_WARN("unknown saturation event!?"); 31 } 32 } RBX = 0x2 = REFCOUNT_ADD_UAF, confirming the switch branch taken. 5. kobject_get -- lib/kobject.c:643 636 struct kobject *kobject_get(struct kobject *kobj) 637 { 638 if (kobj) { 639 if (!kobj->state_initialized) 640 WARN(1, KERN_WARNING 641 "kobject: '%s' (%p): is not initialized, " 641 "yet kobject_get() is being called.\n", 642 kobject_name(kobj), kobj); -> 643 kref_get(&kobj->kref); /* unconditional increment */ 644 } 645 return kobj; 646 } kobject_get_unless_zero() (line 649) checks for zero first via kref_get_unless_zero, but is not used here. 6. i2c_get_adapter -- drivers/i2c/i2c-core-base.c:2612 2602 struct i2c_adapter *i2c_get_adapter(int nr) 2603 { 2604 struct i2c_adapter *adapter; 2605 2606 mutex_lock(&core_lock); 2607 adapter = idr_find(&i2c_adapter_idr, nr); 2608 if (!adapter) 2609 goto exit; 2610 2611 if (try_module_get(adapter->owner)) ->2612 get_device(&adapter->dev); /* crash: dev refcount == 0 */ 2613 else 2614 adapter = NULL; 2615 2616 exit: 2617 mutex_unlock(&core_lock); 2618 return adapter; 2619 } 7. i2cdev_open -- drivers/i2c/i2c-dev.c:603 597 static int i2cdev_open(struct inode *inode, struct file *file) 598 { 599 unsigned int minor = iminor(inode); 600 struct i2c_client *client; 601 struct i2c_adapter *adap; 602 -> 603 adap = i2c_get_adapter(minor); 604 if (!adap) 605 return -ENODEV; ... 618 } Tentative Analysis The crash is a "refcount_t: addition on 0; use-after-free" WARNING in refcount_warn_saturate(), triggered when get_device() is called on an I2C adapter whose struct device refcount has already reached zero. The sequence of events: 1. A task opens /dev/i2c-N, which calls i2cdev_open(), which calls i2c_get_adapter(minor). 2. i2c_get_adapter() takes core_lock, calls idr_find(), and finds the adapter in the IDR (non-NULL). try_module_get() succeeds. get_device(&adapter->dev) is then called. 3. Concurrently, i2c_del_adapter() has already called device_unregister(&adap->dev), which drops the device refcount to zero. At this point the adapter is still in the IDR: idr_remove() is not called until after wait_for_completion() returns. 4. There is therefore a window where i2c_get_adapter() can find the adapter in the IDR and attempt get_device() on an object whose kobject refcount is already 0, triggering the WARNING. The race exists because i2c_del_adapter() removes the adapter from the IDR only *after* wait_for_completion(), which itself is called after device_unregister() (which drops the device refcount to zero when there is only one reference). i2c_get_adapter() holds core_lock for its entire body including the get_device() call, but i2c_del_adapter() releases core_lock before calling device_unregister(), so the two can execute concurrently. The bug was introduced by commit 611e12ea0f12 ("i2c: core: manage i2c bus device refcount in i2c_[get|put]_adapter", 2015), which added get_device() to i2c_get_adapter() without reordering the idr_remove() call in i2c_del_adapter(). Before that commit only try_module_get() was called, so the idr_remove() ordering was inconsequential. Potential Solution Move the idr_remove() call in i2c_del_adapter() to before the device_unregister() call, still under core_lock. Once the adapter is removed from the IDR, any concurrent i2c_get_adapter() call will receive NULL from idr_find() and return -ENODEV. Callers that already obtained a device reference before the idr_remove() hold a legitimate reference; wait_for_completion() will correctly wait for them to release it via i2c_put_adapter(). Rough patch (against e753c16cb3dd): --- a/drivers/i2c/i2c-core-base.c +++ b/drivers/i2c/i2c-core-base.c @@ i2c_del_adapter + /* Remove from IDR before device_unregister() to prevent a + * concurrent i2c_get_adapter() from calling get_device() on a + * kobject whose refcount has already reached zero. + */ + mutex_lock(&core_lock); + idr_remove(&i2c_adapter_idr, adap->nr); + mutex_unlock(&core_lock); + /* wait until all references to the device are gone ... */ init_completion(&adap->dev_released); 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); - More information Oops-Analysis: http://oops.fenrus.org/reports/lkml/69e93024.a00a0220.17a17.0031.GAE_google.com/ Assisted-by: GitHub Copilot linux-kernel-oops-x86.