linux-kernel - Re: [PATCH v7 3/5] KVM: selftests: Add atoi_paranoid() to catch errors missed by atoi()

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y2AfUKJ19yZrlHzN@google.com>
Date:   Mon, 31 Oct 2022 19:17:36 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     Vipin Sharma <vipinsh@...gle.com>
Cc:     pbonzini@...hat.com, dmatlack@...gle.com, andrew.jones@...ux.dev,
        wei.w.wang@...el.com, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v7 3/5] KVM: selftests: Add atoi_paranoid() to catch
 errors missed by atoi()

On Mon, Oct 31, 2022, Vipin Sharma wrote:
> atoi() doesn't detect errors. There is no way to know that a 0 return
> is correct conversion or due to an error.
> 
> Introduce atoi_paranoid() to detect errors and provide correct
> conversion. Replace all atoi() calls with atoi_paranoid().
> diff --git a/tools/testing/selftests/kvm/lib/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c
> index 6d23878bbfe1..ec0f070a6f21 100644
> --- a/tools/testing/selftests/kvm/lib/test_util.c
> +++ b/tools/testing/selftests/kvm/lib/test_util.c
> @@ -334,3 +334,22 @@ long get_run_delay(void)
>  
>  	return val[1];
>  }
> +
> +int atoi_paranoid(const char *num_str)
> +{
> +	char *end_ptr;
> +	long num;
> +
> +	errno = 0;
> +	num = strtol(num_str, &end_ptr, 10);

I take back my review.  This forces specifying params in decimal, e.g. a large
hex number yields:

  strtol("0xffffffffff") failed to parse trailing characters "xffffffffff".

Obviously I'm intentionally being a bad user in this particular case, but there
will inevitably be tests that want to take hex input, e.g. an x86 test that takes
an MSR index would definitely want hex input.

Looking through all selftests, I don't think there are existing cases that would
likely want hex, but it's trivial to support since strtol() will autodetect the
format if the base is '0', i.e.

diff --git a/tools/testing/selftests/kvm/lib/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c
index 210e98a49a83..8c37cfa12edc 100644
--- a/tools/testing/selftests/kvm/lib/test_util.c
+++ b/tools/testing/selftests/kvm/lib/test_util.c
@@ -341,7 +341,7 @@ int atoi_paranoid(const char *num_str)
        long num;
 
        errno = 0;
-       num = strtol(num_str, &end_ptr, 10);
+       num = strtol(num_str, &end_ptr, 0);
        TEST_ASSERT(!errno, "strtol(\"%s\") failed", num_str);
        TEST_ASSERT(num_str != end_ptr,
                    "strtol(\"%s\") didn't find a valid integer.\n", num_str);