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

["Naladala, Ramanaidu" <Ramanaidu.naladala@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 2495 bytes --]


On 9/24/2026 9:06 PM, sashiko-bot@kernel.org wrote:
> 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?
This is a debugfs-only control intended for user space use. We generally 
don't provide strong concurrency guarantees for multiple user space 
writers racing on debugfs interfaces. The actual GMBUS state changes are 
already serialized inside |intel_gmbus_force_bit(). |
>
> 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;
>> +}
> [ ... ]
>

[-- Attachment #2: Type: text/html, Size: 3693 bytes --]