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

Deepanshu Kartikey posted 1 patch 2 days ago
block/ioctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] block: add allocation size check in blkdev_pr_read_keys()
Posted by Deepanshu Kartikey 2 days ago
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()
Posted by Christoph Hellwig 1 day, 19 hours ago
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()
Posted by Deepanshu Kartikey 1 day, 10 hours ago
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;
 }