Re: [PATCH] i2c: core: fix debugfs UAF on adapter removal

[Vasileios Almpanis]

On 7/31/26 11:35 PM, Andi Shyti wrote:
Hi Andi,
> Hi Vasileios,
>
> ...
>
> > diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> > index 3ec04787a737..b894563f5a75 100644
> > --- a/drivers/i2c/i2c-core-base.c
> > +++ b/drivers/i2c/i2c-core-base.c
> > @@ -1826,8 +1826,6 @@ void i2c_del_adapter(struct i2c_adapter *adap)
> >   
> >   	i2c_host_notify_irq_teardown(adap);
> >   
> > -	debugfs_remove_recursive(adap->debugfs);
> > -
> >   	/* wait until all references to the device are gone
> >   	 *
> >   	 * FIXME: This is old code and should ideally be replaced by an
> > @@ -1839,6 +1837,9 @@ void i2c_del_adapter(struct i2c_adapter *adap)
> >   	device_unregister(&adap->dev);
> >   	wait_for_completion(&adap->dev_released);
> >   
> > +	/* clients use this directory as their debugfs parent */
> > +	debugfs_remove_recursive(adap->debugfs);
> > +
> Speaking of sysfs, this can't work if a new device is created
> through the new_device interface. Perhaps you can remove the
> attribute first with device_remove_file(), but we need to check
> whether that could lead to a double removal when the device
> attributes are cleaned up during device removal.
>
> Thanks,
> Andi
Thanks you for reviewing my patch. You're right. A write to new_device
that happens during removing clients leaks a client. It holds a reference
on the adapter device, so wait_for_completion() never returns.
I reproduced this locally by adding mdelay(5000) right after 
unregistering clients
and creating a client. The write succeeds and i2c_del_adapter() hangs.

I already wrote another patch that includes my hunk from v1 but also adds
device_remove_file(&adap->dev, &dev_attr_new_device) before deregistering
clients as you suggested. This no longer reproduces the same bug.
I also tested it with CONFIG_DEBUG_KOBJECT_RELEASE and KASAN as
i2c_del_adapter suggests and no warnings were emitted.

syzbot also tested the result against mainline and the reproducer
doesn't trigger anymore.

Is it okay with you if I keep the hunk of v1? Or you prefer its left as 
originally,
since after removing the new_device attr, we can't have new clients so
leaving there is safe.

Thanks,
Vasilis
> >   	/* free bus id */
> >   	mutex_lock(&core_lock);
> >   	idr_remove(&i2c_adapter_idr, adap->nr);
> > --
> > 2.47.3