efi/efi_test: bound capsule_count to what the int loop index can hold

[PATCH] efi/efi_test: bound capsule_count to what the int loop index can hold

Posted by Muhammad Bilal 5 days, 2 hours ago

efi_runtime_query_capsulecaps() only rejects capsule_count == ULONG_MAX
(to stop "capsule_count + 1" wrapping the kzalloc_objs() count to
zero), but then walks the array with
"for (i = 0; i < qcaps.capsule_count; i++)"
using a plain int i against an unsigned long bound. A capsule_count
between INT_MAX and ULONG_MAX - 1 lets i wrap through INT_MIN instead
of ever reaching the loop bound, and capsules[i] with a negative i
indexes before the allocation.

kzalloc_objs() would have to succeed at that size for the loop to be
reached at all, which bounds this in practice, but the check should
not rely on the allocator failing first. Reject any capsule_count
that would not fit in the int index up front.

Fixes: 092e72c9edab ("efi/efi_test: Prevent an Oops in efi_runtime_query_capsulecaps()")
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
 drivers/firmware/efi/test/efi_test.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/test/efi_test.c b/drivers/firmware/efi/test/efi_test.c
index d54d6a671326..683a0524dd31 100644
--- a/drivers/firmware/efi/test/efi_test.c
+++ b/drivers/firmware/efi/test/efi_test.c
@@ -611,7 +611,8 @@ static long efi_runtime_query_capsulecaps(unsigned long arg)
 	if (copy_from_user(&qcaps, qcaps_user, sizeof(qcaps)))
 		return -EFAULT;
 
-	if (qcaps.capsule_count == ULONG_MAX)
+	/* capsule_count is iterated over with a signed int index below */
+	if (qcaps.capsule_count >= INT_MAX)
 		return -EINVAL;
 
 	capsules = kzalloc_objs(efi_capsule_header_t, qcaps.capsule_count + 1);
-- 
2.55.0

Re: [PATCH] efi/efi_test: bound capsule_count to what the int loop index can hold

Posted by Dan Carpenter 3 days, 8 hours ago

On Sun, Sep 20, 2026 at 12:24:10AM +0500, Muhammad Bilal wrote:
> efi_runtime_query_capsulecaps() only rejects capsule_count == ULONG_MAX
> (to stop "capsule_count + 1" wrapping the kzalloc_objs() count to
> zero), but then walks the array with
> "for (i = 0; i < qcaps.capsule_count; i++)"
> using a plain int i against an unsigned long bound. A capsule_count
> between INT_MAX and ULONG_MAX - 1 lets i wrap through INT_MIN instead
> of ever reaching the loop bound, and capsules[i] with a negative i
> indexes before the allocation.
> 
> kzalloc_objs() would have to succeed at that size for the loop to be
> reached at all, which bounds this in practice, but the check should
> not rely on the allocator failing first. Reject any capsule_count
> that would not fit in the int index up front.

No, the it's fine to rely on kzalloc failure.  Don't bother trying
to silence this false positive.  Fix your checker instead.

> 
> Fixes: 092e72c9edab ("efi/efi_test: Prevent an Oops in efi_runtime_query_capsulecaps()")

Certainly, don't add a Fixes tag.

> Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
> ---

Regards,
dan carpenter