1. Patchew
  2. linux
  3. [PATCH v2 0/4] block: add IOC_PR_READ_KEYS and IOC_PR_READ_RESERVATION ioctls
  4. View patch

[PATCH v2 2/4] nvme: reject invalid pr_read_keys() num_keys values

Stefan Hajnoczi posted 4 patches 2 months, 1 week ago
There is a newer version of this series
[PATCH v2 2/4] nvme: reject invalid pr_read_keys() num_keys values
Posted by Stefan Hajnoczi 2 months, 1 week ago 
The pr_read_keys() interface has a u32 num_keys parameter. The NVMe
Reservation Report command has a u32 maximum length. Reject num_keys
values that are too large to fit.

This will become important when pr_read_keys() is exposed to untrusted
userspace via an <linux/pr.h> ioctl.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 drivers/nvme/host/pr.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/nvme/host/pr.c b/drivers/nvme/host/pr.c
index ca6a74607b139..156a2ae1fac2e 100644
--- a/drivers/nvme/host/pr.c
+++ b/drivers/nvme/host/pr.c
@@ -233,6 +233,10 @@ static int nvme_pr_read_keys(struct block_device *bdev,
 	int ret, i;
 	bool eds;
 
+	/* Check that keys fit into u32 rse_len */
+	if (num_keys > (U32_MAX - sizeof(*rse)) / sizeof(rse->regctl_eds[0]))
+		return -EINVAL;
+
 	/*
 	 * Assume we are using 128-bit host IDs and allocate a buffer large
 	 * enough to get enough keys to fill the return keys buffer.
-- 
2.52.0
Re: [PATCH v2 2/4] nvme: reject invalid pr_read_keys() num_keys values
Posted by Chaitanya Kulkarni 2 months, 1 week ago 
On 11/27/25 07:54, Stefan Hajnoczi wrote:
> The pr_read_keys() interface has a u32 num_keys parameter. The NVMe
> Reservation Report command has a u32 maximum length. Reject num_keys
> values that are too large to fit.
>
> This will become important when pr_read_keys() is exposed to untrusted
> userspace via an <linux/pr.h> ioctl.
>
> Signed-off-by: Stefan Hajnoczi<stefanha@redhat.com>
> ---
>   drivers/nvme/host/pr.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/drivers/nvme/host/pr.c b/drivers/nvme/host/pr.c
> index ca6a74607b139..156a2ae1fac2e 100644
> --- a/drivers/nvme/host/pr.c
> +++ b/drivers/nvme/host/pr.c
> @@ -233,6 +233,10 @@ static int nvme_pr_read_keys(struct block_device *bdev,
>   	int ret, i;
>   	bool eds;
>   
> +	/* Check that keys fit into u32 rse_len */
> +	if (num_keys > (U32_MAX - sizeof(*rse)) / sizeof(rse->regctl_eds[0]))
> +		return -EINVAL;

de-referencing res in res->regctl_eds[0] is safe in this patch ?

if so please ignore this comment ...

-ck
Re: [PATCH v2 2/4] nvme: reject invalid pr_read_keys() num_keys values
Posted by Christoph Hellwig 2 months, 1 week ago 
On Mon, Dec 01, 2025 at 07:11:31AM +0000, Chaitanya Kulkarni wrote:
> On 11/27/25 07:54, Stefan Hajnoczi wrote:
> > The pr_read_keys() interface has a u32 num_keys parameter. The NVMe
> > Reservation Report command has a u32 maximum length. Reject num_keys
> > values that are too large to fit.
> >
> > This will become important when pr_read_keys() is exposed to untrusted
> > userspace via an <linux/pr.h> ioctl.
> >
> > Signed-off-by: Stefan Hajnoczi<stefanha@redhat.com>
> > ---
> >   drivers/nvme/host/pr.c | 4 ++++
> >   1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/nvme/host/pr.c b/drivers/nvme/host/pr.c
> > index ca6a74607b139..156a2ae1fac2e 100644
> > --- a/drivers/nvme/host/pr.c
> > +++ b/drivers/nvme/host/pr.c
> > @@ -233,6 +233,10 @@ static int nvme_pr_read_keys(struct block_device *bdev,
> >   	int ret, i;
> >   	bool eds;
> >   
> > +	/* Check that keys fit into u32 rse_len */
> > +	if (num_keys > (U32_MAX - sizeof(*rse)) / sizeof(rse->regctl_eds[0]))
> > +		return -EINVAL;
> 
> de-referencing res in res->regctl_eds[0] is safe in this patch ?

sizeof does not dereference pointers in the expression.
Re: [PATCH v2 2/4] nvme: reject invalid pr_read_keys() num_keys values
Posted by Christoph Hellwig 2 months, 1 week ago 
On Thu, Nov 27, 2025 at 10:54:22AM -0500, Stefan Hajnoczi wrote:
> The pr_read_keys() interface has a u32 num_keys parameter. The NVMe
> Reservation Report command has a u32 maximum length. Reject num_keys
> values that are too large to fit.
> 
> This will become important when pr_read_keys() is exposed to untrusted
> userspace via an <linux/pr.h> ioctl.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  drivers/nvme/host/pr.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/nvme/host/pr.c b/drivers/nvme/host/pr.c
> index ca6a74607b139..156a2ae1fac2e 100644
> --- a/drivers/nvme/host/pr.c
> +++ b/drivers/nvme/host/pr.c
> @@ -233,6 +233,10 @@ static int nvme_pr_read_keys(struct block_device *bdev,
>  	int ret, i;
>  	bool eds;
>  
> +	/* Check that keys fit into u32 rse_len */
> +	if (num_keys > (U32_MAX - sizeof(*rse)) / sizeof(rse->regctl_eds[0]))
> +		return -EINVAL;
> +

We use struct_size to calculate the size below, which saturates on
overflow.  So just checking the rse_len variable returned by the that
would be nicer.  Bonus points for using sizeof_field() instead of
hardcoding U32_MAX.
Re: [PATCH v2 2/4] nvme: reject invalid pr_read_keys() num_keys values
Posted by Stefan Hajnoczi 2 months, 1 week ago 
On Mon, Dec 01, 2025 at 07:36:49AM +0100, Christoph Hellwig wrote:
> On Thu, Nov 27, 2025 at 10:54:22AM -0500, Stefan Hajnoczi wrote:
> > The pr_read_keys() interface has a u32 num_keys parameter. The NVMe
> > Reservation Report command has a u32 maximum length. Reject num_keys
> > values that are too large to fit.
> > 
> > This will become important when pr_read_keys() is exposed to untrusted
> > userspace via an <linux/pr.h> ioctl.
> > 
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> >  drivers/nvme/host/pr.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/nvme/host/pr.c b/drivers/nvme/host/pr.c
> > index ca6a74607b139..156a2ae1fac2e 100644
> > --- a/drivers/nvme/host/pr.c
> > +++ b/drivers/nvme/host/pr.c
> > @@ -233,6 +233,10 @@ static int nvme_pr_read_keys(struct block_device *bdev,
> >  	int ret, i;
> >  	bool eds;
> >  
> > +	/* Check that keys fit into u32 rse_len */
> > +	if (num_keys > (U32_MAX - sizeof(*rse)) / sizeof(rse->regctl_eds[0]))
> > +		return -EINVAL;
> > +
> 
> We use struct_size to calculate the size below, which saturates on
> overflow.  So just checking the rse_len variable returned by the that
> would be nicer.  Bonus points for using sizeof_field() instead of
> hardcoding U32_MAX.

Will fix. I don't see how to use sizeof_field() here, but taking
advantage of struct_size() already improves things a lot:

diff --git a/drivers/nvme/host/pr.c b/drivers/nvme/host/pr.c
index ca6a74607b139..ad2ecc2f49a97 100644
--- a/drivers/nvme/host/pr.c
+++ b/drivers/nvme/host/pr.c
@@ -228,7 +228,8 @@ static int nvme_pr_resv_report(struct block_device *bdev, void *data,
 static int nvme_pr_read_keys(struct block_device *bdev,
                struct pr_keys *keys_info)
 {
-       u32 rse_len, num_keys = keys_info->num_keys;
+       size_t rse_len;
+       u32 num_keys = keys_info->num_keys;
        struct nvme_reservation_status_ext *rse;
        int ret, i;
        bool eds;
@@ -238,6 +239,9 @@ static int nvme_pr_read_keys(struct block_device *bdev,
         * enough to get enough keys to fill the return keys buffer.
         */
        rse_len = struct_size(rse, regctl_eds, num_keys);
+       if (rse_len > U32_MAX)
+               return -EINVAL;
+
        rse = kzalloc(rse_len, GFP_KERNEL);
        if (!rse)
                return -ENOMEM;

Stefan
Re: [PATCH v2 2/4] nvme: reject invalid pr_read_keys() num_keys values
Posted by Christoph Hellwig 2 months, 1 week ago 
On Mon, Dec 01, 2025 at 11:22:55AM -0500, Stefan Hajnoczi wrote:
> > We use struct_size to calculate the size below, which saturates on
> > overflow.  So just checking the rse_len variable returned by the that
> > would be nicer.  Bonus points for using sizeof_field() instead of
> > hardcoding U32_MAX.
> 
> Will fix. I don't see how to use sizeof_field() here, but taking
> advantage of struct_size() already improves things a lot:

I thought we'd stuff the len in some field, but we actually convert
it to the ndw in the command, so yes it doesn't make sense here.
Sorry for the misleading direction.
Re: [PATCH v2 2/4] nvme: reject invalid pr_read_keys() num_keys values
Posted by Hannes Reinecke 2 months, 1 week ago 
On 11/27/25 16:54, Stefan Hajnoczi wrote:
> The pr_read_keys() interface has a u32 num_keys parameter. The NVMe
> Reservation Report command has a u32 maximum length. Reject num_keys
> values that are too large to fit.
> 
> This will become important when pr_read_keys() is exposed to untrusted
> userspace via an <linux/pr.h> ioctl.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>   drivers/nvme/host/pr.c | 4 ++++
>   1 file changed, 4 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

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.