KVM: selftests: Fix the never-true negative UFFD delay check

[PATCH] KVM: selftests: Fix the never-true negative UFFD delay check

Posted by Chaithanya Lagisetty 2 weeks, 4 days ago

demand_paging_test parses the -d option with strtoul() and then asserts
that the result is not negative:

	p.uffd_delay = strtoul(optarg, NULL, 0);
	TEST_ASSERT(p.uffd_delay >= 0, "A negative UFFD delay is not supported.");

p.uffd_delay is a useconds_t, which is an unsigned type, so the comparison
is always true and the assertion can never fire. GCC points this out with
-Wtype-limits, which is enabled by -Wextra.

As a result, "-d -1" is accepted and converted to a very large unsigned
delay, causing each demand paging fault to sleep for an unexpectedly long
time in usleep() instead of rejecting the argument up front. The return
value of strtoul() is not validated either, so a non-numeric argument
such as "-d abc" is silently treated as a zero delay.

Use atoi_non_negative() instead. It rejects negative values, unparsable
input, and trailing garbage. It is already used a few lines below for -v,
and hexadecimal input keeps working because atoi_paranoid() also passes a
base of 0 to strtol().

Fixes: 0119cb365c93 ("KVM: selftests: Add configurable demand paging delay")
Signed-off-by: Chaithanya Lagisetty <nagachaithanya9911@gmail.com>
---
 tools/testing/selftests/kvm/demand_paging_test.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
index f8b3d0b68830..619a0b2be45c 100644
--- a/tools/testing/selftests/kvm/demand_paging_test.c
+++ b/tools/testing/selftests/kvm/demand_paging_test.c
@@ -297,8 +297,7 @@ int main(int argc, char *argv[])
 			p.single_uffd = true;
 			break;
 		case 'd':
-			p.uffd_delay = strtoul(optarg, NULL, 0);
-			TEST_ASSERT(p.uffd_delay >= 0, "A negative UFFD delay is not supported.");
+			p.uffd_delay = atoi_non_negative("UFFD delay", optarg);
 			break;
 		case 'b':
 			guest_percpu_mem_size = parse_size(optarg);
-- 
2.43.0

Re: [PATCH] KVM: selftests: Fix the never-true negative UFFD delay check

Posted by Sean Christopherson 4 days, 9 hours ago

On Mon, 07 Sep 2026 04:52:46 +0000, Chaithanya Lagisetty wrote:
> demand_paging_test parses the -d option with strtoul() and then asserts
> that the result is not negative:
> 
> 	p.uffd_delay = strtoul(optarg, NULL, 0);
> 	TEST_ASSERT(p.uffd_delay >= 0, "A negative UFFD delay is not supported.");
> 
> p.uffd_delay is a useconds_t, which is an unsigned type, so the comparison
> is always true and the assertion can never fire. GCC points this out with
> -Wtype-limits, which is enabled by -Wextra.
> 
> [...]

Applied to kvm-x86 selftests, thanks!

[1/1] KVM: selftests: Fix the never-true negative UFFD delay check
      https://github.com/kvm-x86/linux/commit/25158cac2d15

--
https://github.com/kvm-x86/linux/tree/next

Re: [PATCH] KVM: selftests: Fix the never-true negative UFFD delay check

Posted by Gautam Menghani 2 weeks, 2 days ago

On Mon, Sep 07, 2026 at 04:52:46AM +0000, Chaithanya Lagisetty wrote:
> demand_paging_test parses the -d option with strtoul() and then asserts
> that the result is not negative:
> 
> 	p.uffd_delay = strtoul(optarg, NULL, 0);
> 	TEST_ASSERT(p.uffd_delay >= 0, "A negative UFFD delay is not supported.");
> 
> p.uffd_delay is a useconds_t, which is an unsigned type, so the comparison
> is always true and the assertion can never fire. GCC points this out with
> -Wtype-limits, which is enabled by -Wextra.
> 
> As a result, "-d -1" is accepted and converted to a very large unsigned
> delay, causing each demand paging fault to sleep for an unexpectedly long
> time in usleep() instead of rejecting the argument up front. The return
> value of strtoul() is not validated either, so a non-numeric argument
> such as "-d abc" is silently treated as a zero delay.
> 
> Use atoi_non_negative() instead. It rejects negative values, unparsable
> input, and trailing garbage. It is already used a few lines below for -v,
> and hexadecimal input keeps working because atoi_paranoid() also passes a
> base of 0 to strtol().
> 
> Fixes: 0119cb365c93 ("KVM: selftests: Add configurable demand paging delay")
> Signed-off-by: Chaithanya Lagisetty <nagachaithanya9911@gmail.com>
> ---
>  tools/testing/selftests/kvm/demand_paging_test.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
> index f8b3d0b68830..619a0b2be45c 100644
> --- a/tools/testing/selftests/kvm/demand_paging_test.c
> +++ b/tools/testing/selftests/kvm/demand_paging_test.c
> @@ -297,8 +297,7 @@ int main(int argc, char *argv[])
>  			p.single_uffd = true;
>  			break;
>  		case 'd':
> -			p.uffd_delay = strtoul(optarg, NULL, 0);
> -			TEST_ASSERT(p.uffd_delay >= 0, "A negative UFFD delay is not supported.");
> +			p.uffd_delay = atoi_non_negative("UFFD delay", optarg);
>  			break;
>  		case 'b':
>  			guest_percpu_mem_size = parse_size(optarg);
> -- 
> 2.43.0
> 

With this patch applied, the test fails as intended:

# ./demand_paging_test -d -1
Random seed: 0x236c0ed9

==== Test Assertion Failure ====
  include/test_util.h:236: num >= 0
  pid=3674565 tid=3674565 errno=0 - Success
     1  0x00000000004010d0: atoi_non_negative at test_util.h:236
     2   (inlined by) main at demand_paging_test.c:300
     3  0x00007f907d9f05b4: ?? ??:0
     4  0x00007f907d9f0667: ?? ??:0
     5  0x0000000000401374: _start at ??:?
  UFFD delay must be non-negative, got '-1'


Tested-by: Gautam Menghani <gautam@linux.ibm.com>
Reviewed-by: Gautam Menghani <gautam@linux.ibm.com>