Add a Persistent Reservations ioctl to read the list of currently
registered reservation keys. This calls the pr_ops->read_keys() function
that was previously added in commit c787f1baa503 ("block: Add PR
callouts for read keys and reservation") but was only used by the
in-kernel SCSI target so far.

The IOC_PR_READ_KEYS ioctl is necessary so that userspace applications
that rely on Persistent Reservations ioctls have a way of inspecting the
current state. Cluster managers and validation tests need this
functionality.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/uapi/linux/pr.h |  7 +++++
 block/ioctl.c           | 59 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+)

diff --git a/include/uapi/linux/pr.h b/include/uapi/linux/pr.h
index d8126415966f3..fcb74eab92c80 100644
--- a/include/uapi/linux/pr.h
+++ b/include/uapi/linux/pr.h
@@ -56,6 +56,12 @@ struct pr_clear {
 	__u32	__pad;
 };
 
+struct pr_read_keys {
+	__u32	generation;
+	__u32	num_keys;
+	__u64	keys_ptr;
+};
+
 #define PR_FL_IGNORE_KEY	(1 << 0)	/* ignore existing key */
 
 #define IOC_PR_REGISTER		_IOW('p', 200, struct pr_registration)
@@ -64,5 +70,6 @@ struct pr_clear {
 #define IOC_PR_PREEMPT		_IOW('p', 203, struct pr_preempt)
 #define IOC_PR_PREEMPT_ABORT	_IOW('p', 204, struct pr_preempt)
 #define IOC_PR_CLEAR		_IOW('p', 205, struct pr_clear)
+#define IOC_PR_READ_KEYS	_IOWR('p', 206, struct pr_read_keys)
 
 #endif /* _UAPI_PR_H */
diff --git a/block/ioctl.c b/block/ioctl.c
index d7489a56b33c3..63b942392b234 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <linux/capability.h>
+#include <linux/cleanup.h>
 #include <linux/compat.h>
 #include <linux/blkdev.h>
 #include <linux/export.h>
@@ -423,6 +424,62 @@ static int blkdev_pr_clear(struct block_device *bdev, blk_mode_t mode,
 	return ops->pr_clear(bdev, c.key);
 }
 
+static int blkdev_pr_read_keys(struct block_device *bdev, blk_mode_t mode,
+		struct pr_read_keys __user *arg)
+{
+	const struct pr_ops *ops = bdev->bd_disk->fops->pr_ops;
+	struct pr_keys *keys_info __free(kfree) = NULL;
+	struct pr_read_keys inout;
+	u64 __user *keys_ptr;
+	size_t keys_info_len;
+	size_t keys_copy_len;
+	u32 num_copy_keys;
+	int ret;
+
+	if (!blkdev_pr_allowed(bdev, mode))
+		return -EPERM;
+	if (!ops || !ops->pr_read_keys)
+		return -EOPNOTSUPP;
+
+	if (copy_from_user(&inout, arg, sizeof(inout)))
+		return -EFAULT;
+
+	/*
+	 * 64-bit hosts could handle more keys than 32-bit hosts, but this
+	 * limit is more than enough in practice.
+	 */
+	if (inout.num_keys > (U32_MAX - sizeof(*keys_info)) /
+	                     sizeof(keys_info->keys[0]))
+		return -EINVAL;
+
+	keys_info_len = struct_size(keys_info, keys, inout.num_keys);
+	keys_info = kzalloc(keys_info_len, GFP_KERNEL);
+	if (!keys_info)
+		return -ENOMEM;
+
+	keys_info->num_keys = inout.num_keys;
+
+	ret = ops->pr_read_keys(bdev, keys_info);
+	if (ret)
+		return ret;
+
+	/* Copy out individual keys */
+	keys_ptr = u64_to_user_ptr(inout.keys_ptr);
+	num_copy_keys = min(inout.num_keys, keys_info->num_keys);
+	keys_copy_len = num_copy_keys * sizeof(keys_info->keys[0]);
+
+	if (copy_to_user(keys_ptr, keys_info->keys, keys_copy_len))
+		return -EFAULT;
+
+	/* Copy out the arg struct */
+	inout.generation = keys_info->generation;
+	inout.num_keys = keys_info->num_keys;
+
+	if (copy_to_user(arg, &inout, sizeof(inout)))
+		return -EFAULT;
+	return ret;
+}
+
 static int blkdev_flushbuf(struct block_device *bdev, unsigned cmd,
 		unsigned long arg)
 {
@@ -644,6 +701,8 @@ static int blkdev_common_ioctl(struct block_device *bdev, blk_mode_t mode,
 		return blkdev_pr_preempt(bdev, mode, argp, true);
 	case IOC_PR_CLEAR:
 		return blkdev_pr_clear(bdev, mode, argp);
+	case IOC_PR_READ_KEYS:
+		return blkdev_pr_read_keys(bdev, mode, argp);
 	default:
 		return blk_get_meta_cap(bdev, cmd, argp);
 	}
-- 
2.52.0

On Thu, Nov 27, 2025 at 10:54:23AM -0500, Stefan Hajnoczi wrote:
> +static int blkdev_pr_read_keys(struct block_device *bdev, blk_mode_t mode,
> +		struct pr_read_keys __user *arg)
> +{
> +	const struct pr_ops *ops = bdev->bd_disk->fops->pr_ops;
> +	struct pr_keys *keys_info __free(kfree) = NULL;

Please avoid the use of the __free mess and write readable and maintainable
code instead.

> +	struct pr_read_keys inout;

Inout is not a very good variable name, as it doesn't really have much
of meaning.  

> +	if (copy_from_user(&inout, arg, sizeof(inout)))
> +		return -EFAULT;
> +
> +	/*
> +	 * 64-bit hosts could handle more keys than 32-bit hosts, but this
> +	 * limit is more than enough in practice.
> +	 */
> +	if (inout.num_keys > (U32_MAX - sizeof(*keys_info)) /
> +	                     sizeof(keys_info->keys[0]))
> +		return -EINVAL;
> +
> +	keys_info_len = struct_size(keys_info, keys, inout.num_keys);

Do the size check on the calculate len here?

> +		return ret;
> +
> +	/* Copy out individual keys */
> +	keys_ptr = u64_to_user_ptr(inout.keys_ptr);
> +	num_copy_keys = min(inout.num_keys, keys_info->num_keys);
> +	keys_copy_len = num_copy_keys * sizeof(keys_info->keys[0]);

num_copy_keys is only used once, so maybe drop it?

On Mon, Dec 01, 2025 at 07:40:16AM +0100, Christoph Hellwig wrote:
> On Thu, Nov 27, 2025 at 10:54:23AM -0500, Stefan Hajnoczi wrote:
> > +static int blkdev_pr_read_keys(struct block_device *bdev, blk_mode_t mode,
> > +		struct pr_read_keys __user *arg)
> > +{
> > +	const struct pr_ops *ops = bdev->bd_disk->fops->pr_ops;
> > +	struct pr_keys *keys_info __free(kfree) = NULL;
> 
> Please avoid the use of the __free mess and write readable and maintainable
> code instead.

Okay.

> > +	struct pr_read_keys inout;
> 
> Inout is not a very good variable name, as it doesn't really have much
> of meaning.  

It's the ioctl argument. I will change it to read_keys in the next
revision. I'm not sure if that's any better, but it reminds us which
struct this is.

> > +	if (copy_from_user(&inout, arg, sizeof(inout)))
> > +		return -EFAULT;
> > +
> > +	/*
> > +	 * 64-bit hosts could handle more keys than 32-bit hosts, but this
> > +	 * limit is more than enough in practice.
> > +	 */
> > +	if (inout.num_keys > (U32_MAX - sizeof(*keys_info)) /
> > +	                     sizeof(keys_info->keys[0]))
> > +		return -EINVAL;
> > +
> > +	keys_info_len = struct_size(keys_info, keys, inout.num_keys);
> 
> Do the size check on the calculate len here?

Yes, that's better. Checking SIZE_MAX also gets rid of the 32-bit vs
64-bit host comment.

> > +		return ret;
> > +
> > +	/* Copy out individual keys */
> > +	keys_ptr = u64_to_user_ptr(inout.keys_ptr);
> > +	num_copy_keys = min(inout.num_keys, keys_info->num_keys);
> > +	keys_copy_len = num_copy_keys * sizeof(keys_info->keys[0]);
> 
> num_copy_keys is only used once, so maybe drop it?

Will fix.

Stefan

On 11/27/25 16:54, Stefan Hajnoczi wrote:
> Add a Persistent Reservations ioctl to read the list of currently
> registered reservation keys. This calls the pr_ops->read_keys() function
> that was previously added in commit c787f1baa503 ("block: Add PR
> callouts for read keys and reservation") but was only used by the
> in-kernel SCSI target so far.
> 
> The IOC_PR_READ_KEYS ioctl is necessary so that userspace applications
> that rely on Persistent Reservations ioctls have a way of inspecting the
> current state. Cluster managers and validation tests need this
> functionality.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>   include/uapi/linux/pr.h |  7 +++++
>   block/ioctl.c           | 59 +++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 66 insertions(+)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich