lkml.org 
[lkml]   [2017]   [Aug]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 1/3] lib/test_kmod: tidy up bounds checking
On Fri, Jul 07, 2017 at 11:39:33AM +0300, Dan Carpenter wrote:
> There is technically a bug where we don't test for negatives in
> test_dev_config_update_uint_sync(). "new" is long and UINT_MAX is
> unsigned int so on 64 bit systems negatives are allowed.

Good catch. I however prefer we instead just replace kstrtol() with
kstrtoul() in both of these cases as an atomic separate patch with
a Fixes tag. I'll do this and roll in your other patches. Below is
what I mean as one atomic fix.

diff --git a/lib/test_kmod.c b/lib/test_kmod.c
index 90c91541fc16..8fc0a7a19c83 100644
--- a/lib/test_kmod.c
+++ b/lib/test_kmod.c
@@ -880,10 +880,10 @@ static int test_dev_config_update_uint_sync(struct kmod_test_device *test_dev,
int (*test_sync)(struct kmod_test_device *test_dev))
{
int ret;
- long new;
+ unsigned long new;
unsigned int old_val;

- ret = kstrtol(buf, 10, &new);
+ ret = kstrtoul(buf, 10, &new);
if (ret)
return ret;

@@ -918,9 +918,9 @@ static int test_dev_config_update_uint_range(struct kmod_test_device *test_dev,
unsigned int max)
{
int ret;
- long new;
+ unsigned long new;

- ret = kstrtol(buf, 10, &new);
+ ret = kstrtoul(buf, 10, &new);
if (ret)
return ret;

> In the next test I removed the UINT_MAX comparison because "max" is
> already an unsigned int so we already know that "new" can't be larger
> than UINT_MAX.
Makes sense, will roll this in as a separate patch on your part.

> On the third test, I just flipped the tests around so we consistently
> test the lower bound before the upper bound.

And since non-functional I will also split up into another patch.

Luis

\
 
 \ /
  Last update: 2017-08-02 01:11    [W:0.045 / U:43.440 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site