Re: [PATCH v7 08/27] libmultipath: Add support for block device IOCTL

[John Garry <john.garry@linux.dev>]

On 9/1/26 12:04, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] AB-BA deadlock in mpath_bdev_ioctl() due to holding SRCU read lock across synchronous underlying ioctl operations.
> --
> 
> commit 5e662d20d610c2b48599b805387034d536eb95fd
> Author: John Garry <john.garry@linux.dev>
> 
> libmultipath: Add support for block device IOCTL
> 
> This commit adds mpath_bdev_ioctl() as a multipath block device IOCTL
> handler. This handler forwards the call into the underlying mpath_device
> bdev fops handler.
> 
>> diff --git a/lib/multipath.c b/lib/multipath.c
>> index 58eb9be5bebbe..f1758d1e60a60 100644
>> --- a/lib/multipath.c
>> +++ b/lib/multipath.c
> 
> [ ... ]
> 
>> @@ -454,11 +454,53 @@ static void mpath_bdev_release(struct gendisk *disk)
>>   	mpath_put_head(mpath_head);
>>   }
>>   
>> +static int mpath_bdev_ioctl(struct block_device *bdev, blk_mode_t mode,
>> +		    unsigned int cmd, unsigned long arg)
>> +{
> 
> [ ... ]
> 
>> +	srcu_idx = srcu_read_lock(&mpath_head->srcu);
>> +	mpath_device = mpath_find_path(mpath_head);
>> +	if (!mpath_device) {
>> +		err = -EWOULDBLOCK;
>> +		goto out_unlock;
>> +	}
>> +
>> +	if (!mpath_device->disk->fops->ioctl) {
>> +		err = -ENOTTY;
>> +		goto out_unlock;
>> +	}
>> +
>> +	err = mpath_device->disk->fops->ioctl(
>> +			mpath_device->disk->part0, mode, cmd, arg);
>> +out_unlock:
>> +	srcu_read_unlock(&mpath_head->srcu, srcu_idx);
> 
> [Severity: High]
> Can this cause an AB-BA deadlock during device removal?
> 
> If the underlying ioctl (e.g., a passthrough command via SG_IO) blocks
> waiting for I/O completion, and a concurrent device removal is triggered
> (e.g., via physical hot-unplug, sysfs deletion, or SCSI error recovery
> offline), could we deadlock here?

If the device is removed, then the IO should complete with failure and 
we can continue.

> 
> A device removal path typically calls:
> sd_mpath_remove() -> mpath_delete_device() -> synchronize_srcu(&mpath_head->srcu).
> 
> That removal thread would block indefinitely waiting for this ioctl thread
> to release the SRCU read lock. Because the device removal is stalled, it
> cannot proceed to tear down the device and abort the pending I/O, leaving
> this ioctl thread permanently stuck waiting for the I/O.
> 
> Would it be safer to acquire a reference to the underlying disk (e.g., via
> get_disk()) and drop the SRCU lock before invoking the potentially unbounded
> ioctl? This appears to be a known structural constraint in block multipathing,
> similar to a fix previously applied in nvme-multipath.

nvme-multipath ensures that any ioctl which can lead to the device being 
removed is not called with the SRCU lock. Furthermore, in that case, we 
are issuing the ioctl to the nvme controller and not the per-path NS.

> 
>> +	return err;
>> +}
>> +
>