Re: [PATCH v2 1/1] drm/i915/gmbus: Add bit-banging debugfs control

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/i915: race in debugfs write causes force_bit refcount corruption
--

commit dbe416d20c046e80d3b0b6e516a640339bbbd1e9
Author: Naladala Ramanaidu <ramanaidu.naladala@intel.com>

drm/i915/gmbus: Add bit-banging debugfs control

This commit adds a connector debugfs interface to enable or disable I2C
bit-banging for Intel GMBUS adapters.

> diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c b/drivers/gpu/drm/i915/display/intel_gmbus.c
> index 60a70dea5d856..38d6d11483994 100644
> --- a/drivers/gpu/drm/i915/display/intel_gmbus.c
> +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c

[ ... ]

> +static ssize_t intel_force_bit_banging_write(struct file *file,
> +						     const char __user *ubuf,
> +						     size_t len, loff_t *offp)
> +{
> +	struct seq_file *m = file->private_data;
> +	struct intel_connector *connector = m->private;
> +	bool force_bit_banging;
> +	int ret;
> +
> +	ret = kstrtobool_from_user(ubuf, len, &force_bit_banging);
> +	if (ret)
> +		return ret;
> +
> +	if (force_bit_banging != connector->force_bit_banging) {
> +		intel_gmbus_force_bit(connector->base.ddc, force_bit_banging);
> +		connector->force_bit_banging = force_bit_banging;
> +	}

[Severity: Medium]
Is this state transition safe against concurrent debugfs writes?

Since this read-modify-write block on connector->force_bit_banging
is not protected by any locks, multiple threads concurrently writing to
the intel_force_bit_banging debugfs file could trigger multiple calls to
intel_gmbus_force_bit() for the same state change.

Looking at intel_gmbus_force_bit(), it manages bus->force_bit as a
reference count:

drivers/gpu/drm/i915/display/intel_gmbus.c:intel_gmbus_force_bit() {
    ...
    mutex_lock(&display->gmbus.mutex);
    bus->force_bit += force_bit ? 1 : -1;
    ...
}

Could unbalanced calls resulting from a race here cause the refcount to
underflow or leak, permanently forcing bit-banging mode for the adapter?

> +	*offp += len;
> +
> +	return len;
> +}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924152812.439986-1-ramanaidu.naladala@intel.com?part=1