| 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
| ||
|
Message-Id: <20230321122514.1743889-3-mark.rutland@arm.com>
Date: Tue, 21 Mar 2023 12:25:12 +0000
From: Mark Rutland <mark.rutland@....com>
To: linux-kernel@...r.kernel.org
Cc: agordeev@...ux.ibm.com, aou@...s.berkeley.edu, bp@...en8.de,
catalin.marinas@....com, dave.hansen@...ux.intel.com,
davem@...emloft.net, gor@...ux.ibm.com, hca@...ux.ibm.com,
linux-arch@...r.kernel.org, linux@...linux.org.uk,
mark.rutland@....com, mingo@...hat.com, palmer@...belt.com,
paul.walmsley@...ive.com, robin.murphy@....com, tglx@...utronix.de,
torvalds@...ux-foundation.org, viro@...iv.linux.org.uk,
will@...nel.org
Subject: [PATCH v2 2/4] lib: test clear_user()
The clear_user() function follows the same conventions as
copy_{to,from}_user(), and presumably has identical requirements on the
return value. Test it in the same way.
I've given this a spin on a few architectures using the KUnit QEMU
harness, and it looks like most get *something* wrong, or I've
misunderstood and clear_user() doesn't have the same requirements as
copy_{to,from}_user()). From those initial runs:
* arm, arm64, i386, riscv, x86_64 don't ensure that at least 1 byte is
zeroed when a partial zeroing is possible, e.g.
| too few bytes consumed (offset=4095, size=2, ret=2)
| too few bytes consumed (offset=4093, size=4, ret=4)
| too few bytes consumed (offset=4089, size=8, ret=8)
* s390 reports that some bytes have been zeroed even when they haven't,
e.g.
| zeroed bytes incorrect (dst_page[4031+64]=0xca, offset=4031, size=66, ret=1
* sparc passses all tests
Signed-off-by: Mark Rutland <mark.rutland@....com>
Cc: Al Viro <viro@...iv.linux.org.uk>
Cc: Albert Ou <aou@...s.berkeley.edu>
Cc: Alexander Gordeev <agordeev@...ux.ibm.com>
Cc: Borislav Petkov <bp@...en8.de>
Cc: Catalin Marinas <catalin.marinas@....com>
Cc: Dave Hansen <dave.hansen@...ux.intel.com>
Cc: Heiko Carstens <hca@...ux.ibm.com>
Cc: Ingo Molnar <mingo@...hat.com>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Palmer Dabbelt <palmer@...belt.com>
Cc: Paul Walmsley <paul.walmsley@...ive.com>
Cc: Robin Murphy <robin.murphy@....com>
Cc: Russell King <linux@...linux.org.uk>
Cc: Thomas Gleixner <tglx@...utronix.de>
Cc: Vasily Gorbik <gor@...ux.ibm.com>
Cc: Will Deacon <will@...nel.org>
Cc: linux-arch@...r.kernel.org
---
lib/usercopy_kunit.c | 89 ++++++++++++++++++++++++++++++++++++++++----
1 file changed, 82 insertions(+), 7 deletions(-)
diff --git a/lib/usercopy_kunit.c b/lib/usercopy_kunit.c
index 45983952cc079..1ec0d5bbc179a 100644
--- a/lib/usercopy_kunit.c
+++ b/lib/usercopy_kunit.c
@@ -155,6 +155,11 @@ static void usercopy_test_exit(struct kunit *test)
usercopy_env_free(env);
}
+static char buf_zero(int offset)
+{
+ return 0;
+}
+
static char buf_pattern(int offset)
{
return offset & 0xff;
@@ -230,6 +235,7 @@ static void assert_size_valid(struct kunit *test,
static void assert_src_valid(struct kunit *test,
const struct usercopy_params *params,
+ char (*buf_expected)(int),
const char *src, long src_offset,
unsigned long ret)
{
@@ -240,9 +246,10 @@ static void assert_src_valid(struct kunit *test,
* A usercopy MUST NOT modify the source buffer.
*/
for (int i = 0; i < PAGE_SIZE; i++) {
+ char expected = buf_expected(i);
char val = src[i];
- if (val == buf_pattern(i))
+ if (val == expected)
continue;
KUNIT_ASSERT_FAILURE(test,
@@ -253,6 +260,7 @@ static void assert_src_valid(struct kunit *test,
static void assert_dst_valid(struct kunit *test,
const struct usercopy_params *params,
+ char (*buf_expected)(int),
const char *dst, long dst_offset,
unsigned long ret)
{
@@ -263,9 +271,10 @@ static void assert_dst_valid(struct kunit *test,
* A usercopy MUST NOT modify any bytes before the destination buffer.
*/
for (int i = 0; i < dst_offset; i++) {
+ char expected = buf_expected(i);
char val = dst[i];
- if (val == 0)
+ if (val == expected)
continue;
KUNIT_ASSERT_FAILURE(test,
@@ -278,9 +287,10 @@ static void assert_dst_valid(struct kunit *test,
* buffer.
*/
for (int i = dst_offset + size - ret; i < PAGE_SIZE; i++) {
+ char expected = buf_expected(i);
char val = dst[i];
- if (val == 0)
+ if (val == expected)
continue;
KUNIT_ASSERT_FAILURE(test,
@@ -316,6 +326,29 @@ static void assert_copy_valid(struct kunit *test,
}
}
+static void assert_clear_valid(struct kunit *test,
+ const struct usercopy_params *params,
+ const char *dst, long dst_offset,
+ unsigned long ret)
+{
+ const unsigned long size = params->size;
+ const unsigned long offset = params->offset;
+
+ /*
+ * Have we actually zeroed the bytes we expected to?
+ */
+ for (int i = 0; i < params->size - ret; i++) {
+ char dst_val = dst[dst_offset + i];
+
+ if (dst_val == 0)
+ continue;
+
+ KUNIT_ASSERT_FAILURE(test,
+ "zeroed bytes incorrect (dst_page[%ld+%d]=0x%x, offset=%ld, size=%lu, ret=%lu",
+ dst_offset, i, dst_val,
+ offset, size, ret);
+ }
+}
static unsigned long do_copy_to_user(const struct usercopy_env *env,
const struct usercopy_params *params)
{
@@ -344,6 +377,19 @@ static unsigned long do_copy_from_user(const struct usercopy_env *env,
return ret;
}
+static unsigned long do_clear_user(const struct usercopy_env *env,
+ const struct usercopy_params *params)
+{
+ void __user *uptr = (void __user *)UBUF_ADDR_BASE + params->offset;
+ unsigned long ret;
+
+ kthread_use_mm(env->mm);
+ ret = clear_user(uptr, params->size);
+ kthread_unuse_mm(env->mm);
+
+ return ret;
+}
+
/*
* Generate the size and offset combinations to test.
*
@@ -378,8 +424,10 @@ static void test_copy_to_user(struct kunit *test)
ret = do_copy_to_user(env, ¶ms);
assert_size_valid(test, ¶ms, ret);
- assert_src_valid(test, ¶ms, env->kbuf, 0, ret);
- assert_dst_valid(test, ¶ms, env->ubuf, params.offset, ret);
+ assert_src_valid(test, ¶ms, buf_pattern,
+ env->kbuf, 0, ret);
+ assert_dst_valid(test, ¶ms, buf_zero,
+ env->ubuf, params.offset, ret);
assert_copy_valid(test, ¶ms,
env->ubuf, params.offset,
env->kbuf, 0,
@@ -404,8 +452,10 @@ static void test_copy_from_user(struct kunit *test)
ret = do_copy_from_user(env, ¶ms);
assert_size_valid(test, ¶ms, ret);
- assert_src_valid(test, ¶ms, env->ubuf, params.offset, ret);
- assert_dst_valid(test, ¶ms, env->kbuf, 0, ret);
+ assert_src_valid(test, ¶ms, buf_pattern,
+ env->ubuf, params.offset, ret);
+ assert_dst_valid(test, ¶ms, buf_zero,
+ env->kbuf, 0, ret);
assert_copy_valid(test, ¶ms,
env->kbuf, 0,
env->ubuf, params.offset,
@@ -413,9 +463,34 @@ static void test_copy_from_user(struct kunit *test)
}
}
+static void test_clear_user(struct kunit *test)
+{
+ const struct usercopy_env *env = test->priv;
+
+ for_each_size_offset(size, offset) {
+ const struct usercopy_params params = {
+ .size = size,
+ .offset = offset,
+ };
+ unsigned long ret;
+
+ buf_init_pattern(env->ubuf);
+
+ ret = do_clear_user(env, ¶ms);
+
+ assert_size_valid(test, ¶ms, ret);
+ assert_dst_valid(test, ¶ms, buf_pattern,
+ env->ubuf, params.offset, ret);
+ assert_clear_valid(test, ¶ms,
+ env->ubuf, params.offset,
+ ret);
+ }
+}
+
static struct kunit_case usercopy_cases[] = {
KUNIT_CASE(test_copy_to_user),
KUNIT_CASE(test_copy_from_user),
+ KUNIT_CASE(test_clear_user),
{ /* sentinel */ }
};
--
2.30.2
Powered by blists - more mailing lists