Re: [PATCH v3 12/13] libmultipath: Add mpath_bdev_getgeo()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing NULL check for the optional fops->ioctl callback in mpath_bdev_ioctl
--

commit 5104b9e862d1fb81d42be96c9762fa0a9897083a
Author: John Garry <john.g.garry@oracle.com>

libmultipath: Add mpath_bdev_getgeo()

This patch adds mpath_bdev_getgeo() as a multipath block device .getgeo
handler. It redirects the call into the selected mpath_device disk's
fops->getgeo handler.

> diff --git a/lib/multipath.c b/lib/multipath.c
> index 4e4b347875500..26d9b17998d66 100644
> --- a/lib/multipath.c
> +++ b/lib/multipath.c
> @@ -547,6 +547,26 @@ static int mpath_bdev_ioctl(struct block_device *bdev, blk_mode_t mode,
>  	return err;
>  }
>  
> +static int mpath_bdev_getgeo(struct gendisk *disk, struct hd_geometry *geo)
> +{
> +	struct mpath_head *mpath_head = mpath_gendisk_to_head(disk);
> +	int srcu_idx, ret = -EWOULDBLOCK;
> +	struct mpath_device *mpath_device;
> +
> +	srcu_idx = srcu_read_lock(&mpath_head->srcu);
> +	mpath_device = mpath_find_path(mpath_head);
> +	if (mpath_device) {
> +		if (mpath_device->disk->fops->getgeo)

[Severity: High]
This is a pre-existing issue, but while reviewing this patch I noticed that
the new mpath_bdev_getgeo() correctly checks if the callback is present
before executing it.

However, the pre-existing mpath_bdev_ioctl() function located just above
this code does an unconditional dereference of the ioctl callback:

lib/multipath.c:mpath_bdev_ioctl() {
    ...
    err = mpath_device->disk->fops->ioctl(
            mpath_device->disk->part0, mode, cmd, arg);
    ...
}

If a user-space program issues an ioctl to the multipath block device and
the underlying device driver does not implement the optional ioctl callback,
could this result in a NULL pointer dereference and system panic?

> +			ret = mpath_device->disk->fops->getgeo(
> +					mpath_device->disk, geo);
> +		else
> +			ret = -ENOTTY; /* See blkdev_getgeo */
> +	}
> +	srcu_read_unlock(&mpath_head->srcu, srcu_idx);
> +
> +	return ret;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703102918.3723667-1-john.g.garry@oracle.com?part=12