In KUNIT_EXPECT_MEMEQ and KUNIT_EXPECT_MEMNEQ, add check if one of the inputs is NULL and fail if this is the case.
Currently, the kernel crashes if one of the inputs is NULL. Instead, fail the test and add an appropriate error message.
This was found by the kernel test robot: https://lore.kernel.org/all/202212191448.D6EDPdOh-lkp@intel.com/
Reported-by: kernel test robot lkp@intel.com
Signed-off-by: Rae Moar rmoar@google.com --- include/kunit/test.h | 7 ++++--- lib/kunit/assert.c | 40 +++++++++++++++++++++++++--------------- 2 files changed, 29 insertions(+), 18 deletions(-)
diff --git a/include/kunit/test.h b/include/kunit/test.h index 87ea90576b50..3c7045e22512 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -683,9 +683,10 @@ do { \ .right_text = #right, \ }; \ \ - if (likely(memcmp(__left, __right, __size) op 0)) \ - break; \ - \ + if (likely(__left && __right)) \ + if (likely(memcmp(__left, __right, __size) op 0)) \ + break; \ + \ _KUNIT_FAILED(test, \ assert_type, \ kunit_mem_assert, \ diff --git a/lib/kunit/assert.c b/lib/kunit/assert.c index f5b50babe38d..05a09652f5a1 100644 --- a/lib/kunit/assert.c +++ b/lib/kunit/assert.c @@ -241,24 +241,34 @@ void kunit_mem_assert_format(const struct kunit_assert *assert, mem_assert = container_of(assert, struct kunit_mem_assert, assert);
- string_stream_add(stream, - KUNIT_SUBTEST_INDENT "Expected %s %s %s, but\n", - mem_assert->text->left_text, - mem_assert->text->operation, - mem_assert->text->right_text); + if (!mem_assert->left_value) { + string_stream_add(stream, + KUNIT_SUBTEST_INDENT "Expected %s is not null, but is\n", + mem_assert->text->left_text); + } else if (!mem_assert->right_value) { + string_stream_add(stream, + KUNIT_SUBTEST_INDENT "Expected %s is not null, but is\n", + mem_assert->text->right_text); + } else { + string_stream_add(stream, + KUNIT_SUBTEST_INDENT "Expected %s %s %s, but\n", + mem_assert->text->left_text, + mem_assert->text->operation, + mem_assert->text->right_text);
- string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s ==\n", - mem_assert->text->left_text); - kunit_assert_hexdump(stream, mem_assert->left_value, - mem_assert->right_value, mem_assert->size); + string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s ==\n", + mem_assert->text->left_text); + kunit_assert_hexdump(stream, mem_assert->left_value, + mem_assert->right_value, mem_assert->size);
- string_stream_add(stream, "\n"); + string_stream_add(stream, "\n");
- string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s ==\n", - mem_assert->text->right_text); - kunit_assert_hexdump(stream, mem_assert->right_value, - mem_assert->left_value, mem_assert->size); + string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s ==\n", + mem_assert->text->right_text); + kunit_assert_hexdump(stream, mem_assert->right_value, + mem_assert->left_value, mem_assert->size);
- kunit_assert_print_msg(message, stream); + kunit_assert_print_msg(message, stream); + } } EXPORT_SYMBOL_GPL(kunit_mem_assert_format);
base-commit: 5cb26c298ffde271d9bd1dd1b87ad028218f77fe
On Wed, 25 Jan 2023 at 14:19, Rae Moar rmoar@google.com wrote:
In KUNIT_EXPECT_MEMEQ and KUNIT_EXPECT_MEMNEQ, add check if one of the inputs is NULL and fail if this is the case.
Currently, the kernel crashes if one of the inputs is NULL. Instead, fail the test and add an appropriate error message.
This was found by the kernel test robot: https://lore.kernel.org/all/202212191448.D6EDPdOh-lkp@intel.com/
Reported-by: kernel test robot lkp@intel.com
Signed-off-by: Rae Moar rmoar@google.com
This looks good to me, modulo a small formatting issue with the continuation backslashes (see below).
It might be worth considering this as a fix to the patch which first introduced the MEMEQ macros with a Fixes tag, e.g.: Fixes: b8a926bea8b1 ("kunit: Introduce KUNIT_EXPECT_MEMEQ and KUNIT_EXPECT_MEMNEQ macros")
Otherwise, this is: Reviewed-by: David Gow davidgow@google.com
Cheers, -- David
include/kunit/test.h | 7 ++++--- lib/kunit/assert.c | 40 +++++++++++++++++++++++++--------------- 2 files changed, 29 insertions(+), 18 deletions(-)
diff --git a/include/kunit/test.h b/include/kunit/test.h index 87ea90576b50..3c7045e22512 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -683,9 +683,10 @@ do { \ .right_text = #right, \ }; \ \
if (likely(memcmp(__left, __right, __size) op 0)) \
break; \
\
if (likely(__left && __right)) \
if (likely(memcmp(__left, __right, __size) op 0)) \
break; \
\
The backslashes no longer line up here.
_KUNIT_FAILED(test, \ assert_type, \ kunit_mem_assert, \
diff --git a/lib/kunit/assert.c b/lib/kunit/assert.c index f5b50babe38d..05a09652f5a1 100644 --- a/lib/kunit/assert.c +++ b/lib/kunit/assert.c @@ -241,24 +241,34 @@ void kunit_mem_assert_format(const struct kunit_assert *assert, mem_assert = container_of(assert, struct kunit_mem_assert, assert);
string_stream_add(stream,
KUNIT_SUBTEST_INDENT "Expected %s %s %s, but\n",
mem_assert->text->left_text,
mem_assert->text->operation,
mem_assert->text->right_text);
if (!mem_assert->left_value) {
string_stream_add(stream,
KUNIT_SUBTEST_INDENT "Expected %s is not null, but is\n",
mem_assert->text->left_text);
} else if (!mem_assert->right_value) {
string_stream_add(stream,
KUNIT_SUBTEST_INDENT "Expected %s is not null, but is\n",
mem_assert->text->right_text);
} else {
string_stream_add(stream,
KUNIT_SUBTEST_INDENT "Expected %s %s %s, but\n",
mem_assert->text->left_text,
mem_assert->text->operation,
mem_assert->text->right_text);
string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s ==\n",
mem_assert->text->left_text);
kunit_assert_hexdump(stream, mem_assert->left_value,
mem_assert->right_value, mem_assert->size);
string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s ==\n",
mem_assert->text->left_text);
kunit_assert_hexdump(stream, mem_assert->left_value,
mem_assert->right_value, mem_assert->size);
string_stream_add(stream, "\n");
string_stream_add(stream, "\n");
string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s ==\n",
mem_assert->text->right_text);
kunit_assert_hexdump(stream, mem_assert->right_value,
mem_assert->left_value, mem_assert->size);
string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s ==\n",
mem_assert->text->right_text);
kunit_assert_hexdump(stream, mem_assert->right_value,
mem_assert->left_value, mem_assert->size);
kunit_assert_print_msg(message, stream);
kunit_assert_print_msg(message, stream);
}
} EXPORT_SYMBOL_GPL(kunit_mem_assert_format);
base-commit: 5cb26c298ffde271d9bd1dd1b87ad028218f77fe
2.39.1.405.gd4c25cc71f-goog
linux-kselftest-mirror@lists.linaro.org