Re: [PATCH v2 1/3] i2c: dev: fix blocked adapter deregistration

[Bartosz Golaszewski]

On Thu, Aug 27, 2026 at 12:20:41PM +0200, Johan Hovold wrote:
> The I2C subsystem allows controllers to be used by non-child devices
> that may remain registered after an adapter goes away.
> 
> To handle this, adapter deregistration blocks until the last reference
> to the adapter is released. Albeit unorthodox, this is mostly fine for
> the vast majority of controllers but can cause some trouble when
> controllers reside on hotpluggable buses.
> 
> Specifically, userspace can prevent an adapter from being deregistered
> indefinitely by holding an i2c-dev character device file open. And with
> USB attached controllers this prevents further hub events from being
> processed by the parent hub until the file is closed.
> 
> Fix the i2c-dev implementation by dropping the additional reference
> taken at open() and using an rwsem to make sure the adapter is only
> accessed while registered.
> 
> Note that before commit 611e12ea0f12 ("i2c: core: manage i2c bus device
> refcount in i2c_[get|put]_adapter") an adapter going away would instead
> have resulted in a use-after-free.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Johan Hovold <johan@xxxxxxxxxx>
> ---
>  drivers/i2c/i2c-dev.c | 95 ++++++++++++++++++++++++++++++++++---------
>  1 file changed, 76 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c
> index ccaac5e29f90..0940070c0317 100644
> --- a/drivers/i2c/i2c-dev.c
> +++ b/drivers/i2c/i2c-dev.c
> @@ -28,6 +28,7 @@
>  #include <linux/list.h>
>  #include <linux/module.h>
>  #include <linux/notifier.h>
> +#include <linux/rwsem.h>
>  #include <linux/slab.h>
>  #include <linux/uaccess.h>
>  
> @@ -41,11 +42,17 @@
>   */
>  struct i2c_dev {
>  	struct list_head list;
> +	struct rw_semaphore rwsem;
>  	struct i2c_adapter *adap;
>  	struct device dev;
>  	struct cdev cdev;
>  };
>  
> +struct i2c_dev_data {
> +	struct i2c_dev *i2c_dev;
> +	struct i2c_client client;
> +};
> +
>  #define I2C_MINORS	(MINORMASK + 1)
>  static LIST_HEAD(i2c_dev_list);
>  static DEFINE_SPINLOCK(i2c_dev_list_lock);
> @@ -92,6 +99,11 @@ static void put_i2c_dev(struct i2c_dev *i2c_dev, bool del_cdev)
>  	spin_unlock(&i2c_dev_list_lock);
>  	if (del_cdev)
>  		cdev_device_del(&i2c_dev->cdev, &i2c_dev->dev);
> +
> +	scoped_guard(rwsem_write, &i2c_dev->rwsem) {
> +		i2c_dev->adap = NULL;
> +	}
> +
>  	put_device(&i2c_dev->dev);
>  }
>  
> @@ -134,10 +146,16 @@ ATTRIBUTE_GROUPS(i2c);
>  static ssize_t i2cdev_read(struct file *file, char __user *buf, size_t count,
>  		loff_t *offset)
>  {
> +	struct i2c_dev_data *data = file->private_data;
> +	struct i2c_client *client = &data->client;
> +	struct i2c_dev *i2c_dev = data->i2c_dev;
>  	char *tmp;
>  	int ret;
>  
> -	struct i2c_client *client = file->private_data;
> +	guard(rwsem_read)(&i2c_dev->rwsem);

I think sashiko is correct in its comment and you should limit the
critical section to until right after i2c_master_recv(). Same elsewhere.

Looks good otherwise.

Thanks,
Bartosz

[1] https://sashiko.dev/#/patchset/20260827102043.673273-1-johan%40kernel.org?part=1