Re: [PATCH v9 09/29] libmultipath: Add support for block device IOCTL

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] libmultipath: deadlock in mpath_bdev_ioctl() holding SRCU lock
  across ioctl
--

commit df75a2df3968a2931d7f98024b2fce55905a7703
Author: John Garry <john.garry@linux.dev>

libmultipath: Add support for block device IOCTL

This commit adds mpath_bdev_ioctl() and wires up .ioctl and .compat_ioctl
in mpath_ops to forward block device ioctls to an active path's gendisk.

> diff --git a/lib/multipath.c b/lib/multipath.c
> index ae5a73d570b33..d59d7ba81087f 100644
> --- a/lib/multipath.c
> +++ b/lib/multipath.c
> @@ -455,11 +455,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);
> +out_unlock:
> +	srcu_read_unlock(&mpath_head->srcu, srcu_idx);
> +	return err;
> +}

[Severity: High]
Can holding mpath_head->srcu across fops->ioctl() self-deadlock when the
underlying ioctl synchronously blocks or removes the path?

For example, when fops->ioctl() dispatches to sd_ioctl() -> scsi_ioctl(), a
host reset ioctl (SG_SCSI_RESET) on ib_srp or a host rescan ioctl on
pqi/hpsa can synchronously block or remove the SCSI device in the same
task:

mpath_bdev_ioctl()
  srcu_read_lock(&mpath_head->srcu)
  sd_ioctl()
    scsi_ioctl()
      scsi_ioctl_reset()
        ...
          srp_reset_host()
            srp_reconnect_rport()
              scsi_block_targets()
                ...
                  scsi_device_block()
                    scsi_mpath_dev_clear_path()
                      mpath_synchronize()
                        synchronize_srcu(&mpath_head->srcu)  <-- deadlocks

Waiting for the mpath_head->srcu grace period while the same task holds
srcu_read_lock(&mpath_head->srcu) hangs the caller in uninterruptible sleep
and blocks subsequent path removals and state transitions on mpath_head.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261108204733.557995-1-john.garry@linux.dev?part=9