Re: [PATCH v3 09/13] libmultipath: Add PR support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Calling Persistent Reservation (PR) operations on a multipath partition causes a NULL pointer dereference.
--

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

libmultipath: Add PR support

Add support for persistent reservations.

Effectively all that is done here is that a multipath version of pr_ops is
created which calls into the bdev fops callback for the mpath_device
selected.

> diff --git a/lib/multipath.c b/lib/multipath.c
> index 4945f2d847fbf..cec7047ce7b9b 100644
> --- a/lib/multipath.c
> +++ b/lib/multipath.c
> @@ -491,11 +491,193 @@ static void mpath_bdev_release(struct gendisk *disk)
>  	mpath_put_head(mpath_head);
>  }
>  
> +static int mpath_pr_register(struct block_device *bdev, u64 old_key,
> +			u64 new_key, unsigned int flags)
> +{
> +	struct mpath_head *mpath_head = dev_get_drvdata(&bdev->bd_device);
> +	struct mpath_device *mpath_device;
> +	int srcu_idx, ret = -EWOULDBLOCK;
> +
> +	srcu_idx = srcu_read_lock(&mpath_head->srcu);

[Severity: Critical]
Will this cause a NULL pointer dereference if the persistent reservation
operation is called on a partition?

If the target_core_iblock backend is configured for PR passthrough on a
partition of a multipath device, bdev will be the partition's block_device.
Because dev_set_drvdata() is only called for the base disk's device,
dev_get_drvdata(&bdev->bd_device) will return NULL for partition devices.

Other functions in this file, like mpath_bdev_ioctl(), seem to handle this by
resolving the disk first using mpath_gendisk_to_head(bdev->bd_disk). Should
the PR callbacks do the same?

[ ... ]

> +static int mpath_pr_reserve(struct block_device *bdev, u64 key,
> +		enum pr_type type, unsigned flags)
> +{
> +	struct mpath_head *mpath_head = dev_get_drvdata(&bdev->bd_device);
> +	struct mpath_device *mpath_device;
> +	int srcu_idx, ret = -EWOULDBLOCK;
> +
> +	srcu_idx = srcu_read_lock(&mpath_head->srcu);

[Severity: Critical]
The same NULL pointer dereference risk applies here for partition devices, as
well as to mpath_pr_release(), mpath_pr_preempt(), mpath_pr_clear(),
mpath_pr_read_keys(), and mpath_pr_read_reservation() introduced below.

[ ... ]

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