Re: [PATCH v3 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] i915: gmbus force_bit counter leak via concurrent debugfs writes
--

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

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

This commit adds a debugfs interface, intel_force_bit_banging, to allow
enabling or disabling I2C bit-banging for Intel GMBUS adapters directly
from userspace.

> diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c b/drivers/gpu/drm/i915/display/intel_gmbus.c
> --- a/drivers/gpu/drm/i915/display/intel_gmbus.c
> +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c
[ ... ]
> @@ -871,6 +873,71 @@ static const struct i2c_algorithm gmbus_algorithm = {
[ ... ]
> +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;
> +	struct intel_gmbus *bus = container_of(connector->base.ddc,
> +					       struct intel_gmbus, adapter);
> +	bool force_bit_banging;
> +	int ret;
> +
> +	ret = kstrtobool_from_user(ubuf, len, &force_bit_banging);
> +	if (ret)
> +		return ret;
> +
> +	if (force_bit_banging != bus->debugfs_force_bit) {
> +		intel_gmbus_force_bit(connector->base.ddc, force_bit_banging);
> +		bus->debugfs_force_bit = force_bit_banging;
> +	}

[Severity: Medium]
Could this check-then-act block cause an issue if multiple threads write to
the debugfs file concurrently?

Since there is no lock protecting the read of bus->debugfs_force_bit and
the subsequent update, concurrent writes could see the same value for
bus->debugfs_force_bit and call intel_gmbus_force_bit() multiple times.

Because intel_gmbus_force_bit() manages a reference counter internally:

void intel_gmbus_force_bit(struct i2c_adapter *adapter, bool force_bit)
{
	...
	bus->force_bit += force_bit ? 1 : -1;
	...
}

Redundant calls could permanently leak or underflow the bus->force_bit
counter, locking the GMBUS adapter into bit-banging mode and corrupting
the hardware transfer strategy. While debugfs is typically restricted to
root, a concurrent write sequence could still trigger this.

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

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