Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues: - [High] The static array `soc_gpio_table` caches `devm_`-managed `gpio_desc` pointers, leading to a Use-After-Free across driver reload. - [Medium] Preexisting memory leak of the I2C adapter reference in i2c_acpi_new_device_by_fwnode(). - [Medium] Missing `pinctrl_unregister_mappings()` for `soc_pwm_pinctrl_map` leads to memory leaks and list growth on every driver probe. -- commit 4de09f19213ae7f3a3c535d632462c31364389c5 Author: Ma Ke <[email protected]> drm/i915/dsi: fix i2c adapter reference leak in i2c_adapter_lookup() This commit addresses a reference leak in i2c_adapter_lookup(). It adds a put_device() call to release the adapter returned by i2c_acpi_find_adapter_by_handle(), preventing resource exhaustion over repeated driver load/unload cycles. > diff --git a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c > b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c > index fe12041e913cd..2097c5d17cb7a 100644 > --- a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c > +++ b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c > @@ -460,8 +460,10 @@ static int i2c_adapter_lookup(struct acpi_resource > *ares, void *data) > return 1; > > adapter = i2c_acpi_find_adapter_by_handle(adapter_handle); > - if (adapter) > + if (adapter) { > intel_dsi->i2c_bus_num = adapter->nr; > + put_device(&adapter->dev); > + } > > return 1; > } [Severity: High] This isn't a bug introduced by this patch, but while reviewing this file I noticed a potential use-after-free with soc_gpio_table: drivers/gpu/drm/i915/display/intel_dsi_vbt.c:soc_gpio_set_value() { /* XXX: this table is a quick ugly hack. */ static struct gpio_desc *soc_gpio_table[U8_MAX + 1]; struct gpio_desc *gpio_desc = soc_gpio_table[gpio_index]; if (gpio_desc) { gpiod_set_value(gpio_desc, value); } else { gpio_desc = devm_gpiod_get_index(display->drm->dev, con_id, idx, value ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW); ... soc_gpio_table[gpio_index] = gpio_desc; } } Since soc_gpio_table is static, its lifetime spans across driver unbind and rebind cycles. When the driver is unbound, the devm-managed gpio_desc pointers are freed by the device-managed resource infrastructure. If the driver is later rebound, will soc_gpio_table still hold those dangling pointers? Could this lead to a use-after-free when soc_gpio_set_value() sees gpio_desc is not NULL and passes the freed pointer to gpiod_set_value()? [Severity: Medium] This is another pre-existing issue in the same file. I noticed a potential memory leak with pinctrl_register_mappings(): drivers/gpu/drm/i915/display/intel_dsi_vbt.c:intel_dsi_vbt_gpio_init() { ... if (display->platform.valleyview && mipi_config->pwm_blc == PPS_BLC_SOC) { ... /* Ensure PWM0 pin is muxed as PWM instead of GPIO */ ret = pinctrl_register_mappings(soc_pwm_pinctrl_map, ARRAY_SIZE(soc_pwm_pinctrl_map)); ... } The pinctrl_register_mappings() function dynamically allocates memory for the mappings and adds them to a global list. Since there is no matching pinctrl_unregister_mappings() during driver teardown, and it does not use a devm variant, does this leak memory and indefinitely append duplicate entries to the global list every time the driver is unbound and rebound? [Severity: Medium] This is a pre-existing issue in the I2C ACPI core code that resembles the fix in this patch. Does i2c_acpi_new_device_by_fwnode() also leak the adapter reference? drivers/i2c/i2c-core-acpi.c:i2c_acpi_new_device_by_fwnode() { ... adapter = i2c_acpi_find_adapter_by_handle(lookup.adapter_handle); if (!adapter) return ERR_PTR(-EPROBE_DEFER); return i2c_new_client_device(adapter, info); } The function i2c_acpi_find_adapter_by_handle() returns an i2c_adapter pointer with its device reference count incremented. If i2c_new_client_device() takes its own reference to the adapter during registration but doesn't consume this initial lookup reference, does this cause a permanent reference leak since we return immediately without calling put_device(&adapter->dev)? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
