Skip to content

i2c_dev_delete_mutex never decrements ref_count; master bus leaks on every device delete #16

Description

@UncleRus

Problem

i2c_dev_delete_mutex() never decrements the per-port ref_count and therefore never deletes the master bus. As a result, every call to i2c_dev_delete_mutex() leaks the I2C master bus for that port. The bus can only be cleaned up indirectly via i2cdev_done(), and even then ref_count stays stuck at its last incremented value.

Root cause

In i2cdev.c, i2c_dev_delete_mutex() clears dev->dev_handle before the conditional that is supposed to decrement ref_count:

if (dev->dev_handle)
{
    esp_err_t rm_res = i2c_master_bus_rm_device((i2c_master_dev_handle_t)dev->dev_handle);
    ...
    dev->dev_handle = NULL;          // <-- handle is now NULL
}

// Later, inside the port mutex:
if (i2c_ports[dev->port].installed
    && i2c_ports[dev->port].ref_count > 0
    && dev->dev_handle != NULL) {    // <-- always false
    i2c_ports[dev->port].ref_count--;
    ...
    if (i2c_ports[dev->port].ref_count == 0) {
        i2c_del_master_bus(...);     // <-- never reached for the "real" last device
    }
}

The condition checks dev->dev_handle != NULL, but the field has just been cleared. ref_count is therefore never decremented, and i2c_del_master_bus() is never called from this path.

Secondary issue: race with i2cdev_done()

deregister_device(dev) is called from i2c_dev_delete_mutex() without holding i2c_ports[dev->port].lock, while i2cdev_done() walks active_devices[port][i] under the lock. The two paths can run concurrently on different tasks, leading to a use-after-free on the i2c_dev_t.

Repro

  1. Create two devices on I2C_NUM_0, i2c_dev_create_mutex(&dev1); i2c_dev_create_mutex(&dev2);
  2. Trigger first I2C operation on each so they are actually added to the bus.
  3. Call i2c_dev_delete_mutex(&dev1); — expected: ref_count decrements to 1.
  4. Observe i2c_ports[I2C_NUM_0].ref_count is still 2; bus handle is leaked.

Expected

  • i2c_dev_delete_mutex() decrements ref_count for each device that was actually added to the bus.
  • When ref_count reaches 0, the bus is deleted via i2c_del_master_bus().
  • deregister_device() runs under the same port mutex as i2cdev_done().

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions