[PATCH] block: add allocation size check in blkdev_pr_read_keys()

[PATCH] block: add allocation size check in blkdev_pr_read_keys()

[Deepanshu Kartikey]

blkdev_pr_read_keys() takes num_keys from userspace and uses it to
calculate the allocation size for keys_info via struct_size(). While
there is a check for SIZE_MAX (integer overflow), there is no upper
bound validation on the allocation size itself.

A malicious or buggy userspace can pass a large num_keys value that
doesn't trigger overflow but still results in an excessive allocation
attempt, causing a warning in the page allocator when the order exceeds
MAX_PAGE_ORDER.

Fix this by checking that keys_info_len does not exceed KMALLOC_MAX_SIZE
before attempting the allocation.

Fixes: 22a1ffea5f80 ("block: add IOC_PR_READ_KEYS ioctl")
Reported-by: syzbot+660d079d90f8a1baf54d@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=660d079d90f8a1baf54d
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
 block/ioctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/ioctl.c b/block/ioctl.c
index 61feed686418..3e9e4257569f 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -443,7 +443,7 @@ static int blkdev_pr_read_keys(struct block_device *bdev, blk_mode_t mode,
 		return -EFAULT;
 
 	keys_info_len = struct_size(keys_info, keys, read_keys.num_keys);
-	if (keys_info_len == SIZE_MAX)
+	if (keys_info_len == SIZE_MAX || keys_info_len > KMALLOC_MAX_SIZE)
 		return -EINVAL;
 
 	keys_info = kzalloc(keys_info_len, GFP_KERNEL);
-- 
2.43.0

Re: [PATCH] block: add allocation size check in blkdev_pr_read_keys()

[Christoph Hellwig]

On Fri, Dec 12, 2025 at 07:05:10AM +0530, Deepanshu Kartikey wrote:
>  	keys_info_len = struct_size(keys_info, keys, read_keys.num_keys);
> -	if (keys_info_len == SIZE_MAX)
> +	if (keys_info_len == SIZE_MAX || keys_info_len > KMALLOC_MAX_SIZE)

Well, bother checks for sure are redundant.  But maybe this is also
a good chance to pick a sane arbitrary limited instead of
KMALLOC_MAX_SIZE.  And if that is above KMALLOC_MAX_SIZE, switch to
using kvzalloc.

Re: [PATCH] block: add allocation size check in blkdev_pr_read_keys()

[Deepanshu Kartikey]

On Fri, Dec 12, 2025 at 11:46 AM Christoph Hellwig <hch@infradead.org> wrote:
> Well, bother checks for sure are redundant.  But maybe this is also
> a good chance to pick a sane arbitrary limited instead of
> KMALLOC_MAX_SIZE.  And if that is above KMALLOC_MAX_SIZE, switch to
> using kvzalloc.
>

Thanks for the feedback.

How about limiting num_keys to 64K (1u << 16)? In practice, PR keys
are used for shared storage coordination and typical deployments have
only a handful of hosts, so this should be more than enough for any
realistic use case.

With a bounded num_keys, the SIZE_MAX check becomes unnecessary, so
I've removed it. Also switched to kvzalloc/kvfree to handle larger
allocations gracefully.

Something like below:

+/* Limit the number of keys to prevent excessive memory allocation */
+#define PR_KEYS_MAX_NUM (1u << 16)
+
 static int blkdev_pr_read_keys(struct block_device *bdev, blk_mode_t mode,
  struct pr_read_keys __user *arg)
 {
...

  if (copy_from_user(&read_keys, arg, sizeof(read_keys)))
  return -EFAULT;

+ if (read_keys.num_keys > PR_KEYS_MAX_NUM)
+ return -EINVAL;
+
  keys_info_len = struct_size(keys_info, keys, read_keys.num_keys);
- if (keys_info_len == SIZE_MAX)
- return -EINVAL;

- keys_info = kzalloc(keys_info_len, GFP_KERNEL);
+ keys_info = kvzalloc(keys_info_len, GFP_KERNEL);
  if (!keys_info)
  return -ENOMEM;

...

 out:
- kfree(keys_info);
+ kvfree(keys_info);
  return ret;
 }

Re: [PATCH] block: add allocation size check in blkdev_pr_read_keys()

[Christoph Hellwig]

On Fri, Dec 12, 2025 at 08:17:43PM +0530, Deepanshu Kartikey wrote:
> How about limiting num_keys to 64K (1u << 16)? In practice, PR keys
> are used for shared storage coordination and typical deployments have
> only a handful of hosts, so this should be more than enough for any
> realistic use case.
> 
> With a bounded num_keys, the SIZE_MAX check becomes unnecessary, so
> I've removed it. Also switched to kvzalloc/kvfree to handle larger
> allocations gracefully.
> 
> Something like below:
> 
> +/* Limit the number of keys to prevent excessive memory allocation */
> +#define PR_KEYS_MAX_NUM (1u << 16)

Looks reasonable to me.  Stefan?

Re: [PATCH] block: add allocation size check in blkdev_pr_read_keys()

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 833 bytes --]

On Sun, Dec 14, 2025 at 09:33:59PM -0800, Christoph Hellwig wrote:
> On Fri, Dec 12, 2025 at 08:17:43PM +0530, Deepanshu Kartikey wrote:
> > How about limiting num_keys to 64K (1u << 16)? In practice, PR keys
> > are used for shared storage coordination and typical deployments have
> > only a handful of hosts, so this should be more than enough for any
> > realistic use case.
> > 
> > With a bounded num_keys, the SIZE_MAX check becomes unnecessary, so
> > I've removed it. Also switched to kvzalloc/kvfree to handle larger
> > allocations gracefully.
> > 
> > Something like below:
> > 
> > +/* Limit the number of keys to prevent excessive memory allocation */
> > +#define PR_KEYS_MAX_NUM (1u << 16)
> 
> Looks reasonable to me.  Stefan?

Yes, that's good. Thanks for looking into this, Deepanshu.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]