2022-09-15 15:10:28

by Alexander Potapenko

[permalink] [raw]
Subject: [PATCH v7 18/43] instrumented.h: add KMSAN support

To avoid false positives, KMSAN needs to unpoison the data copied from
the userspace. To detect infoleaks - check the memory buffer passed to
copy_to_user().

Signed-off-by: Alexander Potapenko <[email protected]>
Reviewed-by: Marco Elver <[email protected]>

---
v2:
-- move implementation of kmsan_copy_to_user() here

v5:
-- simplify kmsan_copy_to_user()
-- provide instrument_get_user() and instrument_put_user()

v6:
-- rebase after changing "x86: asm: instrument usercopy in get_user()
and put_user()"

Link: https://linux-review.googlesource.com/id/I43e93b9c02709e6be8d222342f1b044ac8bdbaaf
---
include/linux/instrumented.h | 18 ++++++++++++-----
include/linux/kmsan-checks.h | 19 ++++++++++++++++++
mm/kmsan/hooks.c | 38 ++++++++++++++++++++++++++++++++++++
3 files changed, 70 insertions(+), 5 deletions(-)

diff --git a/include/linux/instrumented.h b/include/linux/instrumented.h
index 9f1dba8f717b0..501fa84867494 100644
--- a/include/linux/instrumented.h
+++ b/include/linux/instrumented.h
@@ -2,7 +2,7 @@

/*
* This header provides generic wrappers for memory access instrumentation that
- * the compiler cannot emit for: KASAN, KCSAN.
+ * the compiler cannot emit for: KASAN, KCSAN, KMSAN.
*/
#ifndef _LINUX_INSTRUMENTED_H
#define _LINUX_INSTRUMENTED_H
@@ -10,6 +10,7 @@
#include <linux/compiler.h>
#include <linux/kasan-checks.h>
#include <linux/kcsan-checks.h>
+#include <linux/kmsan-checks.h>
#include <linux/types.h>

/**
@@ -117,6 +118,7 @@ instrument_copy_to_user(void __user *to, const void *from, unsigned long n)
{
kasan_check_read(from, n);
kcsan_check_read(from, n);
+ kmsan_copy_to_user(to, from, n, 0);
}

/**
@@ -151,6 +153,7 @@ static __always_inline void
instrument_copy_from_user_after(const void *to, const void __user *from,
unsigned long n, unsigned long left)
{
+ kmsan_unpoison_memory(to, n - left);
}

/**
@@ -162,10 +165,14 @@ instrument_copy_from_user_after(const void *to, const void __user *from,
*
* @to destination variable, may not be address-taken
*/
-#define instrument_get_user(to) \
-({ \
+#define instrument_get_user(to) \
+({ \
+ u64 __tmp = (u64)(to); \
+ kmsan_unpoison_memory(&__tmp, sizeof(__tmp)); \
+ to = __tmp; \
})

+
/**
* instrument_put_user() - add instrumentation to put_user()-like macros
*
@@ -177,8 +184,9 @@ instrument_copy_from_user_after(const void *to, const void __user *from,
* @ptr userspace pointer to copy to
* @size number of bytes to copy
*/
-#define instrument_put_user(from, ptr, size) \
-({ \
+#define instrument_put_user(from, ptr, size) \
+({ \
+ kmsan_copy_to_user(ptr, &from, sizeof(from), 0); \
})

#endif /* _LINUX_INSTRUMENTED_H */
diff --git a/include/linux/kmsan-checks.h b/include/linux/kmsan-checks.h
index a6522a0c28df9..c4cae333deec5 100644
--- a/include/linux/kmsan-checks.h
+++ b/include/linux/kmsan-checks.h
@@ -46,6 +46,21 @@ void kmsan_unpoison_memory(const void *address, size_t size);
*/
void kmsan_check_memory(const void *address, size_t size);

+/**
+ * kmsan_copy_to_user() - Notify KMSAN about a data transfer to userspace.
+ * @to: destination address in the userspace.
+ * @from: source address in the kernel.
+ * @to_copy: number of bytes to copy.
+ * @left: number of bytes not copied.
+ *
+ * If this is a real userspace data transfer, KMSAN checks the bytes that were
+ * actually copied to ensure there was no information leak. If @to belongs to
+ * the kernel space (which is possible for compat syscalls), KMSAN just copies
+ * the metadata.
+ */
+void kmsan_copy_to_user(void __user *to, const void *from, size_t to_copy,
+ size_t left);
+
#else

static inline void kmsan_poison_memory(const void *address, size_t size,
@@ -58,6 +73,10 @@ static inline void kmsan_unpoison_memory(const void *address, size_t size)
static inline void kmsan_check_memory(const void *address, size_t size)
{
}
+static inline void kmsan_copy_to_user(void __user *to, const void *from,
+ size_t to_copy, size_t left)
+{
+}

#endif

diff --git a/mm/kmsan/hooks.c b/mm/kmsan/hooks.c
index 6f3e64b0b61f8..5c0eb25d984d7 100644
--- a/mm/kmsan/hooks.c
+++ b/mm/kmsan/hooks.c
@@ -205,6 +205,44 @@ void kmsan_iounmap_page_range(unsigned long start, unsigned long end)
kmsan_leave_runtime();
}

+void kmsan_copy_to_user(void __user *to, const void *from, size_t to_copy,
+ size_t left)
+{
+ unsigned long ua_flags;
+
+ if (!kmsan_enabled || kmsan_in_runtime())
+ return;
+ /*
+ * At this point we've copied the memory already. It's hard to check it
+ * before copying, as the size of actually copied buffer is unknown.
+ */
+
+ /* copy_to_user() may copy zero bytes. No need to check. */
+ if (!to_copy)
+ return;
+ /* Or maybe copy_to_user() failed to copy anything. */
+ if (to_copy <= left)
+ return;
+
+ ua_flags = user_access_save();
+ if ((u64)to < TASK_SIZE) {
+ /* This is a user memory access, check it. */
+ kmsan_internal_check_memory((void *)from, to_copy - left, to,
+ REASON_COPY_TO_USER);
+ } else {
+ /* Otherwise this is a kernel memory access. This happens when a
+ * compat syscall passes an argument allocated on the kernel
+ * stack to a real syscall.
+ * Don't check anything, just copy the shadow of the copied
+ * bytes.
+ */
+ kmsan_internal_memmove_metadata((void *)to, (void *)from,
+ to_copy - left);
+ }
+ user_access_restore(ua_flags);
+}
+EXPORT_SYMBOL(kmsan_copy_to_user);
+
/* Functions from kmsan-checks.h follow. */
void kmsan_poison_memory(const void *address, size_t size, gfp_t flags)
{
--
2.37.2.789.g6183377224-goog


2022-10-19 18:35:02

by Marco Elver

[permalink] [raw]
Subject: Re: [PATCH v7 18/43] instrumented.h: add KMSAN support

On Wed, 19 Oct 2022 at 10:37, youling 257 <[email protected]> wrote:
>
>
>
> ---------- Forwarded message ---------
> 发件人: youling257 <[email protected]>
> Date: 2022年10月20日周四 上午1:36
> Subject: Re: [PATCH v7 18/43] instrumented.h: add KMSAN support
> To: <[email protected]>
> Cc: <[email protected]>
>
>
> i using linux kernel 6.1rc1 on android, i use gcc12 build kernel 6.1 for android, CONFIG_KMSAN is not set.
> "instrumented.h: add KMSAN support" cause android bluetooth high CPU usage.
> git bisect linux kernel 6.1rc1, "instrumented.h: add KMSAN support" is a bad commit for my android.
>
> this is my kernel 6.1, revert include/linux/instrumented.h fix high cpu usage problem.
> https://github.com/youling257/android-mainline/commits/6.1

What arch?
If x86, can you try to revert only the change to
instrument_get_user()? (I wonder if the u64 conversion is causing
issues.)

2022-10-19 19:35:51

by youling 257

[permalink] [raw]
Subject: Re: [PATCH v7 18/43] instrumented.h: add KMSAN support

arch x86, this's my revert,
https://github.com/youling257/android-mainline/commit/401cbfa61cbfc20c87a5be8e2dda68ac5702389f
i tried different revert, have to remove kmsan_copy_to_user.

2022-10-20 1:58 GMT+08:00, Marco Elver <[email protected]>:
> On Wed, 19 Oct 2022 at 10:37, youling 257 <[email protected]> wrote:
>>
>>
>>
>> ---------- Forwarded message ---------
>> 发件人: youling257 <[email protected]>
>> Date: 2022年10月20日周四 上午1:36
>> Subject: Re: [PATCH v7 18/43] instrumented.h: add KMSAN support
>> To: <[email protected]>
>> Cc: <[email protected]>
>>
>>
>> i using linux kernel 6.1rc1 on android, i use gcc12 build kernel 6.1 for
>> android, CONFIG_KMSAN is not set.
>> "instrumented.h: add KMSAN support" cause android bluetooth high CPU
>> usage.
>> git bisect linux kernel 6.1rc1, "instrumented.h: add KMSAN support" is a
>> bad commit for my android.
>>
>> this is my kernel 6.1, revert include/linux/instrumented.h fix high cpu
>> usage problem.
>> https://github.com/youling257/android-mainline/commits/6.1
>
> What arch?
> If x86, can you try to revert only the change to
> instrument_get_user()? (I wonder if the u64 conversion is causing
> issues.)
>

2022-10-19 20:20:10

by Marco Elver

[permalink] [raw]
Subject: Re: [PATCH v7 18/43] instrumented.h: add KMSAN support

On Thu, Oct 20, 2022 at 03:29AM +0800, youling 257 wrote:
[...]
> > What arch?
> > If x86, can you try to revert only the change to
> > instrument_get_user()? (I wonder if the u64 conversion is causing
> > issues.)
> >
> arch x86, this's my revert,
> https://github.com/youling257/android-mainline/commit/401cbfa61cbfc20c87a5be8e2dda68ac5702389f
> i tried different revert, have to remove kmsan_copy_to_user.

There you reverted only instrument_put_user() - does it fix the issue?

If not, can you try only something like this (only revert
instrument_get_user()):

diff --git a/include/linux/instrumented.h b/include/linux/instrumented.h
index 501fa8486749..dbe3ec38d0e6 100644
--- a/include/linux/instrumented.h
+++ b/include/linux/instrumented.h
@@ -167,9 +167,6 @@ instrument_copy_from_user_after(const void *to, const void __user *from,
*/
#define instrument_get_user(to) \
({ \
- u64 __tmp = (u64)(to); \
- kmsan_unpoison_memory(&__tmp, sizeof(__tmp)); \
- to = __tmp; \
})


Once we know which one of these is the issue, we can figure out a proper
fix.

Thanks,

-- Marco

2022-10-19 21:14:33

by youling 257

[permalink] [raw]
Subject: Re: [PATCH v7 18/43] instrumented.h: add KMSAN support

That is i did,i already test, remove "u64 __tmp…kmsan_unpoison_memory", no help.
i only remove kmsan_copy_to_user, fix my issue.

2022-10-20 4:00 GMT+08:00, Marco Elver <[email protected]>:
> On Thu, Oct 20, 2022 at 03:29AM +0800, youling 257 wrote:
> [...]
>> > What arch?
>> > If x86, can you try to revert only the change to
>> > instrument_get_user()? (I wonder if the u64 conversion is causing
>> > issues.)
>> >
>> arch x86, this's my revert,
>> https://github.com/youling257/android-mainline/commit/401cbfa61cbfc20c87a5be8e2dda68ac5702389f
>> i tried different revert, have to remove kmsan_copy_to_user.
>
> There you reverted only instrument_put_user() - does it fix the issue?
>
> If not, can you try only something like this (only revert
> instrument_get_user()):
>
> diff --git a/include/linux/instrumented.h b/include/linux/instrumented.h
> index 501fa8486749..dbe3ec38d0e6 100644
> --- a/include/linux/instrumented.h
> +++ b/include/linux/instrumented.h
> @@ -167,9 +167,6 @@ instrument_copy_from_user_after(const void *to, const
> void __user *from,
> */
> #define instrument_get_user(to) \
> ({ \
> - u64 __tmp = (u64)(to); \
> - kmsan_unpoison_memory(&__tmp, sizeof(__tmp)); \
> - to = __tmp; \
> })
>
>
> Once we know which one of these is the issue, we can figure out a proper
> fix.
>
> Thanks,
>
> -- Marco
>

2022-10-19 21:56:34

by Marco Elver

[permalink] [raw]
Subject: Re: [PATCH v7 18/43] instrumented.h: add KMSAN support

On Thu, Oct 20, 2022 at 04:07AM +0800, youling 257 wrote:
> That is i did,i already test, remove "u64 __tmp…kmsan_unpoison_memory", no help.
> i only remove kmsan_copy_to_user, fix my issue.

Ok - does only the below work (without the reverts)?

diff --git a/include/linux/kmsan-checks.h b/include/linux/kmsan-checks.h
index c4cae333deec..eb05caa8f523 100644
--- a/include/linux/kmsan-checks.h
+++ b/include/linux/kmsan-checks.h
@@ -73,8 +73,8 @@ static inline void kmsan_unpoison_memory(const void *address, size_t size)
static inline void kmsan_check_memory(const void *address, size_t size)
{
}
-static inline void kmsan_copy_to_user(void __user *to, const void *from,
- size_t to_copy, size_t left)
+static __always_inline void kmsan_copy_to_user(void __user *to, const void *from,
+ size_t to_copy, size_t left)
{
}


... because when you say only removing kmsan_copy_to_user() (from
instrument_put_user()) works, it really doesn't make any sense. The only
explanation would be if the compiler inlining is broken.

2022-10-19 22:35:47

by Alexander Potapenko

[permalink] [raw]
Subject: Re: [PATCH v7 18/43] instrumented.h: add KMSAN support

On Wed, Oct 19, 2022 at 1:07 PM youling 257 <[email protected]> wrote:
>
> That is i did,i already test, remove "u64 __tmp…kmsan_unpoison_memory", no help.
> i only remove kmsan_copy_to_user, fix my issue.

Do you have any profiling results that can help pinpoint functions
that are affected by this change?
Also, am I getting it right you are building x86 Android?


> 2022-10-20 4:00 GMT+08:00, Marco Elver <[email protected]>:
> > On Thu, Oct 20, 2022 at 03:29AM +0800, youling 257 wrote:
> > [...]
> >> > What arch?
> >> > If x86, can you try to revert only the change to
> >> > instrument_get_user()? (I wonder if the u64 conversion is causing
> >> > issues.)
> >> >
> >> arch x86, this's my revert,
> >> https://github.com/youling257/android-mainline/commit/401cbfa61cbfc20c87a5be8e2dda68ac5702389f
> >> i tried different revert, have to remove kmsan_copy_to_user.
> >
> > There you reverted only instrument_put_user() - does it fix the issue?
> >
> > If not, can you try only something like this (only revert
> > instrument_get_user()):
> >
> > diff --git a/include/linux/instrumented.h b/include/linux/instrumented.h
> > index 501fa8486749..dbe3ec38d0e6 100644
> > --- a/include/linux/instrumented.h
> > +++ b/include/linux/instrumented.h
> > @@ -167,9 +167,6 @@ instrument_copy_from_user_after(const void *to, const
> > void __user *from,
> > */
> > #define instrument_get_user(to) \
> > ({ \
> > - u64 __tmp = (u64)(to); \
> > - kmsan_unpoison_memory(&__tmp, sizeof(__tmp)); \
> > - to = __tmp; \
> > })
> >
> >
> > Once we know which one of these is the issue, we can figure out a proper
> > fix.
> >
> > Thanks,
> >
> > -- Marco
> >
>
> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/CAOzgRdY6KSxDMRJ%2Bq2BWHs4hRQc5y-PZ2NYG%2B%2B-AMcUrO8YOgA%40mail.gmail.com.



--
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Liana Sebastian
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

2022-10-20 06:45:46

by youling 257

[permalink] [raw]
Subject: Re: [PATCH v7 18/43] instrumented.h: add KMSAN support

without the revert, this include/linux/kmsan-checks.h patch no help.
busybox top, com.android.bluetooth cpu usage 12%.
my kernel config, config-6.1.0-rc1-android-x86_64+

CPU: 3.6% usr 12.1% sys 0.0% nic 84.1% idle 0.0% io 0.0% irq 0.0% sirq
Load average: 1.49 0.65 0.25 4/1336 4940
PID PPID USER STAT VSZ %VSZ CPU %CPU COMMAND
1381 926 1002 S 1265m 16.1 5 12.4 {droid.bluetooth}
com.android.bluetooth
919 1 0 S< 187m 2.4 2 2.2 /system/bin/surfaceflinger
2220 926 10327 S 1882m 24.0 7 1.1 com.baidu.input
3163 926 10175 S 2153m 27.5 0 0.0 {rojekti.clipper}
fi.rojekti.clipper
1250 926 1000 S< 2140m 27.3 0 0.0 system_server
2548 926 10125 S 2078m 26.5 4 0.0 {chiztech.swapps}
com.schiztech.swapps
2516 926 10217 S 2058m 26.3 2 0.0 {ogle.android.gm}
com.google.android.gm
2925 926 10318 S 1876m 24.0 1 0.0 {onelli.juicessh}
com.sonelli.juicessh
926 1 0 S 1780m 22.7 7 0.0 {main} zygote
3374 3369 0 S 1541m 19.7 2 0.0 {main} com.topjohnwu.magisk:root
3079 926 10021 S 1477m 18.9 0 0.0 {ndroid.systemui}
com.android.systemui

2022-10-20 5:36 GMT+08:00, Marco Elver <[email protected]>:
> On Thu, Oct 20, 2022 at 04:07AM +0800, youling 257 wrote:
>> That is i did,i already test, remove "u64 __tmp…kmsan_unpoison_memory", no
>> help.
>> i only remove kmsan_copy_to_user, fix my issue.
>
> Ok - does only the below work (without the reverts)?
>
> diff --git a/include/linux/kmsan-checks.h b/include/linux/kmsan-checks.h
> index c4cae333deec..eb05caa8f523 100644
> --- a/include/linux/kmsan-checks.h
> +++ b/include/linux/kmsan-checks.h
> @@ -73,8 +73,8 @@ static inline void kmsan_unpoison_memory(const void
> *address, size_t size)
> static inline void kmsan_check_memory(const void *address, size_t size)
> {
> }
> -static inline void kmsan_copy_to_user(void __user *to, const void *from,
> - size_t to_copy, size_t left)
> +static __always_inline void kmsan_copy_to_user(void __user *to, const void
> *from,
> + size_t to_copy, size_t left)
> {
> }
>
>
> ... because when you say only removing kmsan_copy_to_user() (from
> instrument_put_user()) works, it really doesn't make any sense. The only
> explanation would be if the compiler inlining is broken.
>


Attachments:
config-6.1.0-rc1-android-x86_64+ (195.22 kB)

2022-10-21 05:58:31

by youling 257

[permalink] [raw]
Subject: Re: [PATCH v7 18/43] instrumented.h: add KMSAN support

How to use perf tool?
I use diff compare "nm -S vmlinux".
android_x86:/ $ su
android_x86:/ # diff /sdcard/with_vmlinux /sdcard/without_vmlinux
--- /sdcard/with_vmlinux
+++ /sdcard/without_vmlinux
@@ -12951,7 +12951,7 @@
ffffffff81338400 00000000000000dc T compat_only_sysfs_link_entry_to_kobj
ffffffff810884d0 00000000000002b7 T compat_ptrace_request
ffffffff812b4e80 0000000000000025 T compat_ptr_ioctl
-ffffffff811380d0 000000000000008d T compat_put_bitmap
+ffffffff811380d0 000000000000008a T compat_put_bitmap
ffffffff81a62180 000000000000000f t compat_raw_ioctl
ffffffff81afa060 000000000000000f t compat_rawv6_ioctl
ffffffff81090790 0000000000000018 T compat_restore_altstack
@@ -14260,7 +14260,7 @@
ffffffff8167d380 00000000000000dd T cppc_set_enable
ffffffff8167d460 0000000000000221 T cppc_set_perf
ffffffff818cdbe0 000000000000004f t cppc_update_perf
-ffffffff8102bba0 000000000000012b t cp_stat64
+ffffffff8102bba0 000000000000012a t cp_stat64
ffffffff81494810 0000000000000028 t cp_status_show
ffffffff812a33e0 00000000000001b2 t cp_statx
ffffffff82e1c934 0000000000000004 b cpu0_hotpluggable
@@ -26993,7 +26993,6 @@
ffffffff821b8000 0000000000000038 d CSWTCH.102
ffffffff821cd6c0 0000000000000048 d CSWTCH.107
ffffffff821d4b80 0000000000000018 d CSWTCH.11
-ffffffff8203dd88 000000000000000c d CSWTCH.111
ffffffff82152de0 00000000000000a0 d CSWTCH.111
ffffffff82105ea0 0000000000000020 d CSWTCH.1116
ffffffff82105e80 0000000000000020 d CSWTCH.1121
@@ -27002,6 +27001,7 @@
ffffffff821d4b60 0000000000000018 d CSWTCH.12
ffffffff821b2c60 0000000000000024 d CSWTCH.120
ffffffff82033fc0 0000000000000068 d CSWTCH.123
+ffffffff8203dd88 000000000000000c d CSWTCH.123
ffffffff82014b80 00000000000000a0 d CSWTCH.125
ffffffff82014b40 0000000000000028 d CSWTCH.128
ffffffff82033f40 0000000000000064 d CSWTCH.128
@@ -27026,7 +27026,6 @@
ffffffff821c9b80 0000000000000018 d CSWTCH.189
ffffffff821b39a0 0000000000000028 d CSWTCH.19
ffffffff820055c0 0000000000000024 d CSWTCH.195
-ffffffff82208de0 0000000000000004 d CSWTCH.199
ffffffff82146127 0000000000000006 d CSWTCH.2
ffffffff82146380 0000000000000014 d CSWTCH.2
ffffffff8219f770 0000000000000018 d CSWTCH.20
@@ -27033,5 +27032,6 @@
ffffffff821d9c90 0000000000000018 d CSWTCH.20
ffffffff821b7e60 0000000000000128 d CSWTCH.202
+ffffffff82208de0 0000000000000004 d CSWTCH.202
ffffffff821b7cc0 0000000000000188 d CSWTCH.204
ffffffff821b7c80 0000000000000038 d CSWTCH.206
ffffffff821a4ac0 0000000000000006 d CSWTCH.207
@@ -27043,8 +27043,8 @@
ffffffff821b7be0 0000000000000038 d CSWTCH.214
ffffffff821c9940 0000000000000030 d CSWTCH.217
ffffffff8219f750 0000000000000018 d CSWTCH.22
-ffffffff82033580 0000000000000023 d CSWTCH.221
ffffffff8213de80 0000000000000028 d CSWTCH.222
+ffffffff82033580 0000000000000023 d CSWTCH.223
ffffffff82216c10 0000000000000014 d CSWTCH.225
ffffffff82216bf0 0000000000000014 d CSWTCH.232
ffffffff82216be0 0000000000000010 d CSWTCH.234
@@ -27052,7 +27052,7 @@
ffffffff821b3800 0000000000000188 d CSWTCH.24
ffffffff821f3340 0000000000000058 d CSWTCH.242
ffffffff82158c60 0000000000000030 d CSWTCH.244
-ffffffff821a9868 000000000000000a d CSWTCH.255
+ffffffff821a9868 000000000000000a d CSWTCH.258
ffffffff82106a20 0000000000000010 d CSWTCH.261
ffffffff821a42a0 0000000000000004 d CSWTCH.261
ffffffff8203d9e0 0000000000000030 d CSWTCH.265
@@ -27059,12 +27059,12 @@
ffffffff82126780 0000000000000028 d CSWTCH.27
ffffffff8214d360 0000000000000040 d CSWTCH.278
ffffffff82136f80 0000000000000024 d CSWTCH.28
-ffffffff821b3000 0000000000000024 d CSWTCH.281
-ffffffff821b2fc0 0000000000000024 d CSWTCH.282
-ffffffff8210ed40 000000000000002c d CSWTCH.289
-ffffffff8210ed00 000000000000002b d CSWTCH.290
-ffffffff8210ecc0 000000000000002c d CSWTCH.292
+ffffffff821b3000 0000000000000024 d CSWTCH.285
+ffffffff821b2fc0 0000000000000024 d CSWTCH.286
+ffffffff8210ed40 000000000000002c d CSWTCH.292
ffffffff821b3ae0 0000000000000020 d CSWTCH.292
+ffffffff8210ed00 000000000000002b d CSWTCH.293
+ffffffff8210ecc0 000000000000002c d CSWTCH.295
ffffffff8214d340 0000000000000020 d CSWTCH.296
ffffffff82146121 0000000000000006 d CSWTCH.3
ffffffff821b7ba0 0000000000000020 d CSWTCH.3
@@ -27075,17 +27075,17 @@
ffffffff821d3c40 0000000000000020 d CSWTCH.31
ffffffff820ff780 0000000000000004 d CSWTCH.318
ffffffff821b5cc0 0000000000000038 d CSWTCH.33
-ffffffff8214efa0 0000000000000010 d CSWTCH.334
+ffffffff8214efa0 0000000000000010 d CSWTCH.346
ffffffff8214a8c0 0000000000000018 d CSWTCH.35
-ffffffff8210ecb0 000000000000000c d CSWTCH.359
-ffffffff8202cae0 000000000000002c d CSWTCH.360
+ffffffff8210ecb0 000000000000000c d CSWTCH.362
ffffffff82016300 0000000000000020 d CSWTCH.363
ffffffff821fce00 0000000000000040 d CSWTCH.38
-ffffffff82032780 0000000000000074 d CSWTCH.387
+ffffffff8202cae0 000000000000002c d CSWTCH.381
+ffffffff82032780 0000000000000074 d CSWTCH.390
ffffffff82118100 0000000000000018 d CSWTCH.40
ffffffff821598a0 0000000000000020 d CSWTCH.41
-ffffffff82032760 0000000000000020 d CSWTCH.428
ffffffff82139a20 0000000000000038 d CSWTCH.43
+ffffffff82032760 0000000000000020 d CSWTCH.431
ffffffff82015d40 0000000000000040 d CSWTCH.45
ffffffff821399e0 0000000000000040 d CSWTCH.45
ffffffff822194e0 000000000000000c d CSWTCH.450
@@ -27092,5 +27092,5 @@
ffffffff8214d2e0 0000000000000048 d CSWTCH.459
-ffffffff8210eca0 000000000000000c d CSWTCH.464
+ffffffff8210eca0 000000000000000c d CSWTCH.467
ffffffff821399c0 0000000000000020 d CSWTCH.47
ffffffff8214a8e0 00000000000000c8 d CSWTCH.48
ffffffff82028ac0 0000000000000018 d CSWTCH.49
@@ -27097,8 +27097,8 @@
ffffffff821399a0 0000000000000020 d CSWTCH.49
-ffffffff82032740 0000000000000018 d CSWTCH.507
-ffffffff82032730 000000000000000c d CSWTCH.508
-ffffffff82032720 000000000000000c d CSWTCH.509
ffffffff82139960 0000000000000030 d CSWTCH.51
+ffffffff82032740 0000000000000018 d CSWTCH.510
+ffffffff82032730 000000000000000c d CSWTCH.511
+ffffffff82032720 000000000000000c d CSWTCH.512
ffffffff821595c0 0000000000000100 d CSWTCH.52
ffffffff821b6240 0000000000000018 d CSWTCH.52
ffffffff821371c0 0000000000000024 d CSWTCH.523
@@ -27125,7 +27125,7 @@
ffffffff82139680 0000000000000058 d CSWTCH.72
ffffffff82139620 0000000000000048 d CSWTCH.74
ffffffff8213c740 0000000000000020 d CSWTCH.74
-ffffffff821f04a0 0000000000000018 d CSWTCH.753
+ffffffff821f04a0 0000000000000018 d CSWTCH.766
ffffffff8200e4e0 0000000000000020 d CSWTCH.77
ffffffff821bffc0 0000000000000018 d CSWTCH.78
ffffffff8200e4c0 0000000000000020 d CSWTCH.79
1|android_x86:/ #

2022-10-21 2:14 GMT+08:00, Alexander Potapenko <[email protected]>:
> On Wed, Oct 19, 2022 at 2:37 PM 'Marco Elver' via kasan-dev <
> [email protected]> wrote:
>
>> On Thu, Oct 20, 2022 at 04:07AM +0800, youling 257 wrote:
>> > That is i did,i already test, remove "u64 __tmp…kmsan_unpoison_memory",
>> no help.
>> > i only remove kmsan_copy_to_user, fix my issue.
>>
>> Ok - does only the below work (without the reverts)?
>>
>> diff --git a/include/linux/kmsan-checks.h b/include/linux/kmsan-checks.h
>> index c4cae333deec..eb05caa8f523 100644
>> --- a/include/linux/kmsan-checks.h
>> +++ b/include/linux/kmsan-checks.h
>> @@ -73,8 +73,8 @@ static inline void kmsan_unpoison_memory(const void
>> *address, size_t size)
>> static inline void kmsan_check_memory(const void *address, size_t size)
>> {
>> }
>> -static inline void kmsan_copy_to_user(void __user *to, const void *from,
>> - size_t to_copy, size_t left)
>> +static __always_inline void kmsan_copy_to_user(void __user *to, const
>> void *from,
>> + size_t to_copy, size_t
>> left)
>> {
>> }
>>
>>
>> ... because when you say only removing kmsan_copy_to_user() (from
>> instrument_put_user()) works, it really doesn't make any sense. The only
>> explanation would be if the compiler inlining is broken.
>>
>>
> If what Marco suggests does not help, could you post the output of `nm -S
> vmlinux` with and without your revert so that we can see which functions
> were affected by the change?
>
> Unfortunately the top results are of no help, do you have the `perf` tool
> available in your system?
>
>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "kasan-dev" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to [email protected].
>> To view this discussion on the web visit
>> https://groups.google.com/d/msgid/kasan-dev/Y1Bt%2BIa93mVV/lT3%40elver.google.com
>> .
>>
>
>
> --
> Alexander Potapenko
> Software Engineer
>
> Google Germany GmbH
> Erika-Mann-Straße, 33
> 80636 München
>
> Geschäftsführer: Paul Manicle, Liana Sebastian
> Registergericht und -nummer: Hamburg, HRB 86891
> Sitz der Gesellschaft: Hamburg
>

2022-10-21 06:48:40

by Marco Elver

[permalink] [raw]
Subject: Re: [PATCH v7 18/43] instrumented.h: add KMSAN support

On Thu, 20 Oct 2022 at 22:55, youling 257 <[email protected]> wrote:
>
> How to use perf tool?

The simplest would be to try just "perf top" - and see which kernel
functions consume most CPU cycles. I would suggest you compare both
kernels, and see if you can spot a function which uses more cycles% in
the problematic kernel.

2022-10-21 06:51:43

by youling 257

[permalink] [raw]
Subject: Re: [PATCH v7 18/43] instrumented.h: add KMSAN support

PerfTop: 8253 irqs/sec kernel:75.3% exact: 100.0% lost: 0/0 drop:
0/17899 [4000Hz cycles], (all, 8 CPUs)
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

14.87% [kernel] [k] 0xffffffff941d1f37
6.71% [kernel] [k] 0xffffffff942016cf

what is 0xffffffff941d1f37?

2022-10-21 14:16 GMT+08:00, Marco Elver <[email protected]>:
> On Thu, 20 Oct 2022 at 22:55, youling 257 <[email protected]> wrote:
>>
>> How to use perf tool?
>
> The simplest would be to try just "perf top" - and see which kernel
> functions consume most CPU cycles. I would suggest you compare both
> kernels, and see if you can spot a function which uses more cycles% in
> the problematic kernel.
>

2022-10-21 08:20:59

by Marco Elver

[permalink] [raw]
Subject: Re: [PATCH v7 18/43] instrumented.h: add KMSAN support

On Thu, 20 Oct 2022 at 23:39, youling 257 <[email protected]> wrote:
>
> PerfTop: 8253 irqs/sec kernel:75.3% exact: 100.0% lost: 0/0 drop:
> 0/17899 [4000Hz cycles], (all, 8 CPUs)
> ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
> 14.87% [kernel] [k] 0xffffffff941d1f37
> 6.71% [kernel] [k] 0xffffffff942016cf
>
> what is 0xffffffff941d1f37?

You need to build with debug symbols:
CONFIG_DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT=y

Then it'll show function names.

> 2022-10-21 14:16 GMT+08:00, Marco Elver <[email protected]>:
> > On Thu, 20 Oct 2022 at 22:55, youling 257 <[email protected]> wrote:
> >>
> >> How to use perf tool?
> >
> > The simplest would be to try just "perf top" - and see which kernel
> > functions consume most CPU cycles. I would suggest you compare both
> > kernels, and see if you can spot a function which uses more cycles% in
> > the problematic kernel.
> >

2022-10-21 15:26:16

by youling 257

[permalink] [raw]
Subject: Re: [PATCH v7 18/43] instrumented.h: add KMSAN support

CONFIG_DEBUG_INFO=y
CONFIG_AS_HAS_NON_CONST_LEB128=y
# CONFIG_DEBUG_INFO_NONE is not set
CONFIG_DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT=y
# CONFIG_DEBUG_INFO_DWARF4 is not set
# CONFIG_DEBUG_INFO_DWARF5 is not set
# CONFIG_DEBUG_INFO_REDUCED is not set
# CONFIG_DEBUG_INFO_COMPRESSED is not set
# CONFIG_DEBUG_INFO_SPLIT is not set
# CONFIG_DEBUG_INFO_BTF is not set
# CONFIG_GDB_SCRIPTS is not set

perf top still no function name.

12.90% [kernel] [k] 0xffffffff833dfa64
3.78% [kernel] [k] 0xffffffff8285b439
3.61% [kernel] [k] 0xffffffff83370254
2.32% [kernel] [k] 0xffffffff8337025b
1.88% bluetooth.default.so [.] 0x000000000000d09d

2022-10-21 15:37 GMT+08:00, Marco Elver <[email protected]>:
> On Thu, 20 Oct 2022 at 23:39, youling 257 <[email protected]> wrote:
>>
>> PerfTop: 8253 irqs/sec kernel:75.3% exact: 100.0% lost: 0/0 drop:
>> 0/17899 [4000Hz cycles], (all, 8 CPUs)
>> ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>>
>> 14.87% [kernel] [k] 0xffffffff941d1f37
>> 6.71% [kernel] [k] 0xffffffff942016cf
>>
>> what is 0xffffffff941d1f37?
>
> You need to build with debug symbols:
> CONFIG_DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT=y
>
> Then it'll show function names.
>
>> 2022-10-21 14:16 GMT+08:00, Marco Elver <[email protected]>:
>> > On Thu, 20 Oct 2022 at 22:55, youling 257 <[email protected]> wrote:
>> >>
>> >> How to use perf tool?
>> >
>> > The simplest would be to try just "perf top" - and see which kernel
>> > functions consume most CPU cycles. I would suggest you compare both
>> > kernels, and see if you can spot a function which uses more cycles% in
>> > the problematic kernel.
>> >
>

2022-10-21 17:42:30

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v7 18/43] instrumented.h: add KMSAN support

On October 21, 2022 10:02:05 AM PDT, Alexander Potapenko <[email protected]> wrote:
>On Fri, Oct 21, 2022 at 8:19 AM youling 257 <[email protected]> wrote:
>
>> CONFIG_DEBUG_INFO=y
>> CONFIG_AS_HAS_NON_CONST_LEB128=y
>> # CONFIG_DEBUG_INFO_NONE is not set
>> CONFIG_DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT=y
>> # CONFIG_DEBUG_INFO_DWARF4 is not set
>> # CONFIG_DEBUG_INFO_DWARF5 is not set
>> # CONFIG_DEBUG_INFO_REDUCED is not set
>> # CONFIG_DEBUG_INFO_COMPRESSED is not set
>> # CONFIG_DEBUG_INFO_SPLIT is not set
>> # CONFIG_DEBUG_INFO_BTF is not set
>> # CONFIG_GDB_SCRIPTS is not set
>>
>> perf top still no function name.
>>
>Will it help if you disable CONFIG_RANDOMIZE_BASE?
>(if it doesn't show the symbols, at least we'll be able to figure out the
>offending function by running nm)

Is KALLSYMS needed?

>
>
>>
>> 12.90% [kernel] [k] 0xffffffff833dfa64
>> 3.78% [kernel] [k] 0xffffffff8285b439
>> 3.61% [kernel] [k] 0xffffffff83370254
>> 2.32% [kernel] [k] 0xffffffff8337025b
>> 1.88% bluetooth.default.so [.] 0x000000000000d09d
>>
>> 2022-10-21 15:37 GMT+08:00, Marco Elver <[email protected]>:
>> > On Thu, 20 Oct 2022 at 23:39, youling 257 <[email protected]> wrote:
>> >>
>> >> PerfTop: 8253 irqs/sec kernel:75.3% exact: 100.0% lost: 0/0 drop:
>> >> 0/17899 [4000Hz cycles], (all, 8 CPUs)
>> >>
>> ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>> >>
>> >> 14.87% [kernel] [k] 0xffffffff941d1f37
>> >> 6.71% [kernel] [k] 0xffffffff942016cf
>> >>
>> >> what is 0xffffffff941d1f37?
>> >
>> > You need to build with debug symbols:
>> > CONFIG_DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT=y
>> >
>> > Then it'll show function names.
>> >
>> >> 2022-10-21 14:16 GMT+08:00, Marco Elver <[email protected]>:
>> >> > On Thu, 20 Oct 2022 at 22:55, youling 257 <[email protected]>
>> wrote:
>> >> >>
>> >> >> How to use perf tool?
>> >> >
>> >> > The simplest would be to try just "perf top" - and see which kernel
>> >> > functions consume most CPU cycles. I would suggest you compare both
>> >> > kernels, and see if you can spot a function which uses more cycles% in
>> >> > the problematic kernel.
>> >> >
>> >
>>
>
>


--
Kees Cook

2022-10-22 06:34:18

by youling 257

[permalink] [raw]
Subject: Re: [PATCH v7 18/43] instrumented.h: add KMSAN support

I test this patch fix my problem.

2022-10-22 4:37 GMT+08:00, Alexander Potapenko <[email protected]>:
> On Fri, Oct 21, 2022 at 8:19 AM youling 257 <[email protected]> wrote:
>
>> CONFIG_DEBUG_INFO=y
>> CONFIG_AS_HAS_NON_CONST_LEB128=y
>> # CONFIG_DEBUG_INFO_NONE is not set
>> CONFIG_DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT=y
>> # CONFIG_DEBUG_INFO_DWARF4 is not set
>> # CONFIG_DEBUG_INFO_DWARF5 is not set
>> # CONFIG_DEBUG_INFO_REDUCED is not set
>> # CONFIG_DEBUG_INFO_COMPRESSED is not set
>> # CONFIG_DEBUG_INFO_SPLIT is not set
>> # CONFIG_DEBUG_INFO_BTF is not set
>> # CONFIG_GDB_SCRIPTS is not set
>>
>> perf top still no function name.
>>
>> 12.90% [kernel] [k] 0xffffffff833dfa64
>>
>
> I think I know what's going on. The two functions that differ with and
> without the patch were passing an incremented pointer to unsafe_put_user(),
> which is a macro, e.g.:
>
> unsafe_put_user((compat_ulong_t)m, umask++, Efault);
>
> Because that macro didn't evaluate its second parameter, "umask++" was
> passed to a call to kmsan_copy_to_user(), which resulted in an extra
> increment of umask.
> This probably violated some expectations of the userspace app, which in
> turn led to repetitive kernel calls.
>
> Could you please check if the patch below fixes the problem for you?
>
> diff --git a/arch/x86/include/asm/uaccess.h
> b/arch/x86/include/asm/uaccess.h
> index 8bc614cfe21b9..1cc756eafa447 100644
> --- a/arch/x86/include/asm/uaccess.h
> +++ b/arch/x86/include/asm/uaccess.h
> @@ -254,24 +254,25 @@ extern void __put_user_nocheck_8(void);
> #define __put_user_size(x, ptr, size, label) \
> do { \
> __typeof__(*(ptr)) __x = (x); /* eval x once */ \
> - __chk_user_ptr(ptr); \
> + __typeof__(ptr) __ptr = (ptr); /* eval ptr once */ \
> + __chk_user_ptr(__ptr); \
> switch (size) { \
> case 1: \
> - __put_user_goto(__x, ptr, "b", "iq", label); \
> + __put_user_goto(__x, __ptr, "b", "iq", label); \
> break; \
> case 2: \
> - __put_user_goto(__x, ptr, "w", "ir", label); \
> + __put_user_goto(__x, __ptr, "w", "ir", label); \
> break; \
> case 4: \
> - __put_user_goto(__x, ptr, "l", "ir", label); \
> + __put_user_goto(__x, __ptr, "l", "ir", label); \
> break; \
> case 8: \
> - __put_user_goto_u64(__x, ptr, label); \
> + __put_user_goto_u64(__x, __ptr, label); \
> break; \
> default: \
> __put_user_bad(); \
> } \
> - instrument_put_user(__x, ptr, size); \
> + instrument_put_user(__x, __ptr, size); \
> } while (0)
>
> #ifdef CONFIG_CC_HAS_ASM_GOTO_OUTPUT
>