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

On 29/07/2026 13:39, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Deadlock in `mpath_bdev_ioctl` due to holding `srcu_read_lock` while forwarding ioctls that can trigger synchronous path removal.
> - [High] Bypass of underlying partition-specific IOCTL security policies because `mpath_bdev_ioctl` unconditionally passes `part0`.
> --
> 
> --- Patch [8]: [PATCH v6 08/27] libmultipath: Add support for block device IOCTL ---
> commit d2e45114ccfb93fccdccc60ed75e05084f9b9173
> Author: John Garry <[email protected]>
> 
> libmultipath: Add support for block device IOCTL
> 
> Add mpath_bdev_ioctl() as a multipath block device IOCTL handler. This
> handler calls into the 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)
>> +{
>> +	struct gendisk *disk = bdev->bd_disk;
>> +	struct mpath_head *mpath_head = mpath_gendisk_to_head(disk);
>> +	struct mpath_device *mpath_device;
>> +	int srcu_idx, err;
>> +
>> +	/*
>> +	 * This check is duplicated from sd_ioctl() as we don't pass the
>> +	 * partition bdev to fops->ioctl. That is not yet possible as the
>> +	 * per-path disk is hidden and, as such, does not have partitions
>> +	 * scanned.
>> +	 */
>> +	if (bdev_is_partition(bdev) && !capable(CAP_SYS_RAWIO))
>> +		return -ENOIOCTLCMD;
>> +
>> +	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);
> 
> [Severity: High]
> Does unconditionally passing part0 bypass partition-specific IOCTL security
> policies in the underlying driver?

Check the comment above regarding sd_ioctl()

> 
> By passing the whole-disk block device (mpath_device->disk->part0),
> bdev_is_partition() will evaluate to false in the underlying driver. For
> drivers like NVMe, this strips the partition context and could bypass
> requirements like CAP_SYS_ADMIN for passthrough commands on partitions,
> allowing a user with only CAP_SYS_RAWIO to execute whole-disk commands.
> 
> [Severity: High]
> Can holding the SRCU read lock across the underlying ioctl cause a deadlock?
> 
> If the underlying ioctl triggers a controller reset or device removal, the
> teardown path could eventually wait on synchronize_srcu() for this exact
> srcu instance. Since the ioctl is waiting for the operation to complete
> while holding srcu_read_lock(), synchronize_srcu() would block forever.
> 

Currently the only ioctls supported cannot support doing controller 
reset or device removal, etc.

>> +out_unlock:
>> +	srcu_read_unlock(&mpath_head->srcu, srcu_idx);
>> +	return err;
>> +}
> 
> [ ... ]
>