From: Arnd Bergmann <[email protected]>
When the compiler decides not to inline this function, objdump
complains about incorrect UACCESS state:
mm/kasan/generic.o: warning: objtool: __asan_load2+0x11: call to addr_has_metadata() with UACCESS enabled
Signed-off-by: Arnd Bergmann <[email protected]>
---
mm/kasan/kasan.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index 3231314e071f..9377b0789edc 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -297,7 +297,7 @@ static inline const void *kasan_shadow_to_mem(const void *shadow_addr)
<< KASAN_SHADOW_SCALE_SHIFT);
}
-static inline bool addr_has_metadata(const void *addr)
+static __always_inline bool addr_has_metadata(const void *addr)
{
return (kasan_reset_tag(addr) >=
kasan_shadow_to_mem((void *)KASAN_SHADOW_START));
@@ -316,7 +316,7 @@ bool kasan_check_range(unsigned long addr, size_t size, bool write,
#else /* CONFIG_KASAN_GENERIC || CONFIG_KASAN_SW_TAGS */
-static inline bool addr_has_metadata(const void *addr)
+static __always_inline bool addr_has_metadata(const void *addr)
{
return (is_vmalloc_addr(addr) || virt_addr_valid(addr));
}
--
2.39.1
From: Arnd Bergmann <[email protected]>
objtool warns about some suspicous code inside of kmsan:
vmlinux.o: warning: objtool: __msan_metadata_ptr_for_load_n+0x4: call to __fentry__() with UACCESS enabled
vmlinux.o: warning: objtool: __msan_metadata_ptr_for_store_n+0x4: call to __fentry__() with UACCESS enabled
vmlinux.o: warning: objtool: __msan_metadata_ptr_for_load_1+0x4: call to __fentry__() with UACCESS enabled
vmlinux.o: warning: objtool: __msan_metadata_ptr_for_store_1+0x4: call to __fentry__() with UACCESS enabled
vmlinux.o: warning: objtool: __msan_metadata_ptr_for_load_2+0x4: call to __fentry__() with UACCESS enabled
vmlinux.o: warning: objtool: __msan_metadata_ptr_for_store_2+0x4: call to __fentry__() with UACCESS enabled
vmlinux.o: warning: objtool: __msan_metadata_ptr_for_load_4+0x4: call to __fentry__() with UACCESS enabled
vmlinux.o: warning: objtool: __msan_metadata_ptr_for_store_4+0x4: call to __fentry__() with UACCESS enabled
vmlinux.o: warning: objtool: __msan_metadata_ptr_for_load_8+0x4: call to __fentry__() with UACCESS enabled
vmlinux.o: warning: objtool: __msan_metadata_ptr_for_store_8+0x4: call to __fentry__() with UACCESS enabled
vmlinux.o: warning: objtool: __msan_instrument_asm_store+0x4: call to __fentry__() with UACCESS enabled
vmlinux.o: warning: objtool: __msan_chain_origin+0x4: call to __fentry__() with UACCESS enabled
vmlinux.o: warning: objtool: __msan_poison_alloca+0x4: call to __fentry__() with UACCESS enabled
vmlinux.o: warning: objtool: __msan_warning+0x4: call to __fentry__() with UACCESS enabled
vmlinux.o: warning: objtool: __msan_get_context_state+0x4: call to __fentry__() with UACCESS enabled
vmlinux.o: warning: objtool: kmsan_copy_to_user+0x4: call to __fentry__() with UACCESS enabled
vmlinux.o: warning: objtool: kmsan_unpoison_memory+0x4: call to __fentry__() with UACCESS enabled
vmlinux.o: warning: objtool: kmsan_unpoison_entry_regs+0x4: call to __fentry__() with UACCESS enabled
vmlinux.o: warning: objtool: kmsan_report+0x4: call to __fentry__() with UACCESS enabled
Similar code already exists in kasan, which avoids this by skipping
ftrace annotations, so do the same thing here.
Fixes: f80be4571b19 ("kmsan: add KMSAN runtime core")
Signed-off-by: Arnd Bergmann <[email protected]>
---
mm/kmsan/Makefile | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/mm/kmsan/Makefile b/mm/kmsan/Makefile
index 98eab2856626..389fd767a11f 100644
--- a/mm/kmsan/Makefile
+++ b/mm/kmsan/Makefile
@@ -16,6 +16,14 @@ CC_FLAGS_KMSAN_RUNTIME += -DDISABLE_BRANCH_PROFILING
CFLAGS_REMOVE.o = $(CC_FLAGS_FTRACE)
+# Disable ftrace to avoid recursion.
+CFLAGS_REMOVE_core.o = $(CC_FLAGS_FTRACE)
+CFLAGS_REMOVE_hooks.o = $(CC_FLAGS_FTRACE)
+CFLAGS_REMOVE_init.o = $(CC_FLAGS_FTRACE)
+CFLAGS_REMOVE_instrumentation.o = $(CC_FLAGS_FTRACE)
+CFLAGS_REMOVE_report.o = $(CC_FLAGS_FTRACE)
+CFLAGS_REMOVE_shadow.o = $(CC_FLAGS_FTRACE)
+
CFLAGS_core.o := $(CC_FLAGS_KMSAN_RUNTIME)
CFLAGS_hooks.o := $(CC_FLAGS_KMSAN_RUNTIME)
CFLAGS_init.o := $(CC_FLAGS_KMSAN_RUNTIME)
--
2.39.1
From: Arnd Bergmann <[email protected]>
The memset/memmove/memcpy string functions are wrapped in different
ways based on configuration. While the __asan_mem* functions already
have exceptions, the ones called from those do not:
mm/kasan/shadow.o: warning: objtool: __asan_memset+0x30: call to __memset() with UACCESS enabled
mm/kasan/shadow.o: warning: objtool: __asan_memmove+0x51: call to __memmove() with UACCESS enabled
mm/kasan/shadow.o: warning: objtool: __asan_memcpy+0x51: call to __memcpy() with UACCESS enabled
vmlinux.o: warning: objtool: .altinstr_replacement+0x1406: call to memcpy_erms() with UACCESS enabled
vmlinux.o: warning: objtool: .altinstr_replacement+0xed0: call to memset_erms() with UACCESS enabled
vmlinux.o: warning: objtool: memset+0x4: call to memset_orig() with UACCESS enabled
vmlinux.o: warning: objtool: memset+0x4: call to memset_orig() with UACCESS enabled
Add these to the list as well.
Signed-off-by: Arnd Bergmann <[email protected]>
---
tools/objtool/check.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 0f67c6a8bc98..e8fb3bf7a2e3 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1248,6 +1248,13 @@ static const char *uaccess_safe_builtin[] = {
"clear_user_erms",
"clear_user_rep_good",
"clear_user_original",
+ "__memset",
+ "__memcpy",
+ "__memmove",
+ "memset_erms",
+ "memcpy_erms",
+ "memset_orig",
+ "memcpy_orig",
NULL
};
--
2.39.1
From: Arnd Bergmann <[email protected]>
A lot of the tsan helpers are already excempt from the UACCESS warnings,
but some more functions were added that need the same thing:
kernel/kcsan/core.o: warning: objtool: __tsan_volatile_read16+0x0: call to __tsan_unaligned_read16() with UACCESS enabled
kernel/kcsan/core.o: warning: objtool: __tsan_volatile_write16+0x0: call to __tsan_unaligned_write16() with UACCESS enabled
vmlinux.o: warning: objtool: __tsan_unaligned_volatile_read16+0x4: call to __tsan_unaligned_read16() with UACCESS enabled
vmlinux.o: warning: objtool: __tsan_unaligned_volatile_write16+0x4: call to __tsan_unaligned_write16() with UACCESS enabled
Fixes: 75d75b7a4d54 ("kcsan: Support distinguishing volatile accesses")
Signed-off-by: Arnd Bergmann <[email protected]>
---
tools/objtool/check.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index e8fb3bf7a2e3..d89ef6957021 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1200,6 +1200,8 @@ static const char *uaccess_safe_builtin[] = {
"__tsan_atomic64_compare_exchange_val",
"__tsan_atomic_thread_fence",
"__tsan_atomic_signal_fence",
+ "__tsan_unaligned_read16",
+ "__tsan_unaligned_write16",
/* KCOV */
"write_comp_data",
"check_kcov_mode",
--
2.39.1
On Wed, 8 Feb 2023 at 17:40, Arnd Bergmann <[email protected]> wrote:
>
> From: Arnd Bergmann <[email protected]>
>
> A lot of the tsan helpers are already excempt from the UACCESS warnings,
> but some more functions were added that need the same thing:
>
> kernel/kcsan/core.o: warning: objtool: __tsan_volatile_read16+0x0: call to __tsan_unaligned_read16() with UACCESS enabled
> kernel/kcsan/core.o: warning: objtool: __tsan_volatile_write16+0x0: call to __tsan_unaligned_write16() with UACCESS enabled
> vmlinux.o: warning: objtool: __tsan_unaligned_volatile_read16+0x4: call to __tsan_unaligned_read16() with UACCESS enabled
> vmlinux.o: warning: objtool: __tsan_unaligned_volatile_write16+0x4: call to __tsan_unaligned_write16() with UACCESS enabled
That's odd - this has never been needed, because all __tsan_unaligned
are aliases for the non-unaligned functions. And all those are in the
uaccess_safe_builtin list already.
So if suddenly the alias name becomes the symbol that objtool sees, we
might need to add all the other functions as well.
Is this a special build with a new compiler?
> Fixes: 75d75b7a4d54 ("kcsan: Support distinguishing volatile accesses")
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> tools/objtool/check.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index e8fb3bf7a2e3..d89ef6957021 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -1200,6 +1200,8 @@ static const char *uaccess_safe_builtin[] = {
> "__tsan_atomic64_compare_exchange_val",
> "__tsan_atomic_thread_fence",
> "__tsan_atomic_signal_fence",
> + "__tsan_unaligned_read16",
> + "__tsan_unaligned_write16",
> /* KCOV */
> "write_comp_data",
> "check_kcov_mode",
> --
> 2.39.1
>
> --
> 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/20230208164011.2287122-4-arnd%40kernel.org.
On Wed, 8 Feb 2023 at 17:40, Arnd Bergmann <[email protected]> wrote:
>
> From: Arnd Bergmann <[email protected]>
>
> objtool warns about some suspicous code inside of kmsan:
>
> vmlinux.o: warning: objtool: __msan_metadata_ptr_for_load_n+0x4: call to __fentry__() with UACCESS enabled
> vmlinux.o: warning: objtool: __msan_metadata_ptr_for_store_n+0x4: call to __fentry__() with UACCESS enabled
> vmlinux.o: warning: objtool: __msan_metadata_ptr_for_load_1+0x4: call to __fentry__() with UACCESS enabled
> vmlinux.o: warning: objtool: __msan_metadata_ptr_for_store_1+0x4: call to __fentry__() with UACCESS enabled
> vmlinux.o: warning: objtool: __msan_metadata_ptr_for_load_2+0x4: call to __fentry__() with UACCESS enabled
> vmlinux.o: warning: objtool: __msan_metadata_ptr_for_store_2+0x4: call to __fentry__() with UACCESS enabled
> vmlinux.o: warning: objtool: __msan_metadata_ptr_for_load_4+0x4: call to __fentry__() with UACCESS enabled
> vmlinux.o: warning: objtool: __msan_metadata_ptr_for_store_4+0x4: call to __fentry__() with UACCESS enabled
> vmlinux.o: warning: objtool: __msan_metadata_ptr_for_load_8+0x4: call to __fentry__() with UACCESS enabled
> vmlinux.o: warning: objtool: __msan_metadata_ptr_for_store_8+0x4: call to __fentry__() with UACCESS enabled
> vmlinux.o: warning: objtool: __msan_instrument_asm_store+0x4: call to __fentry__() with UACCESS enabled
> vmlinux.o: warning: objtool: __msan_chain_origin+0x4: call to __fentry__() with UACCESS enabled
> vmlinux.o: warning: objtool: __msan_poison_alloca+0x4: call to __fentry__() with UACCESS enabled
> vmlinux.o: warning: objtool: __msan_warning+0x4: call to __fentry__() with UACCESS enabled
> vmlinux.o: warning: objtool: __msan_get_context_state+0x4: call to __fentry__() with UACCESS enabled
> vmlinux.o: warning: objtool: kmsan_copy_to_user+0x4: call to __fentry__() with UACCESS enabled
> vmlinux.o: warning: objtool: kmsan_unpoison_memory+0x4: call to __fentry__() with UACCESS enabled
> vmlinux.o: warning: objtool: kmsan_unpoison_entry_regs+0x4: call to __fentry__() with UACCESS enabled
> vmlinux.o: warning: objtool: kmsan_report+0x4: call to __fentry__() with UACCESS enabled
>
> Similar code already exists in kasan, which avoids this by skipping
> ftrace annotations, so do the same thing here.
>
> Fixes: f80be4571b19 ("kmsan: add KMSAN runtime core")
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> mm/kmsan/Makefile | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/mm/kmsan/Makefile b/mm/kmsan/Makefile
> index 98eab2856626..389fd767a11f 100644
> --- a/mm/kmsan/Makefile
> +++ b/mm/kmsan/Makefile
> @@ -16,6 +16,14 @@ CC_FLAGS_KMSAN_RUNTIME += -DDISABLE_BRANCH_PROFILING
>
> CFLAGS_REMOVE.o = $(CC_FLAGS_FTRACE)
That means this CFLAGS_REMOVE.o didn't work, right? Can it be removed?
> +# Disable ftrace to avoid recursion.
> +CFLAGS_REMOVE_core.o = $(CC_FLAGS_FTRACE)
> +CFLAGS_REMOVE_hooks.o = $(CC_FLAGS_FTRACE)
> +CFLAGS_REMOVE_init.o = $(CC_FLAGS_FTRACE)
> +CFLAGS_REMOVE_instrumentation.o = $(CC_FLAGS_FTRACE)
> +CFLAGS_REMOVE_report.o = $(CC_FLAGS_FTRACE)
> +CFLAGS_REMOVE_shadow.o = $(CC_FLAGS_FTRACE)
> +
> CFLAGS_core.o := $(CC_FLAGS_KMSAN_RUNTIME)
> CFLAGS_hooks.o := $(CC_FLAGS_KMSAN_RUNTIME)
> CFLAGS_init.o := $(CC_FLAGS_KMSAN_RUNTIME)
> --
> 2.39.1
>
On Wed, 8 Feb 2023 at 17:40, Arnd Bergmann <[email protected]> wrote:
>
> From: Arnd Bergmann <[email protected]>
>
> When the compiler decides not to inline this function, objdump
> complains about incorrect UACCESS state:
>
> mm/kasan/generic.o: warning: objtool: __asan_load2+0x11: call to addr_has_metadata() with UACCESS enabled
>
> Signed-off-by: Arnd Bergmann <[email protected]>
Reviewed-by: Marco Elver <[email protected]>
> ---
> mm/kasan/kasan.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
> index 3231314e071f..9377b0789edc 100644
> --- a/mm/kasan/kasan.h
> +++ b/mm/kasan/kasan.h
> @@ -297,7 +297,7 @@ static inline const void *kasan_shadow_to_mem(const void *shadow_addr)
> << KASAN_SHADOW_SCALE_SHIFT);
> }
>
> -static inline bool addr_has_metadata(const void *addr)
> +static __always_inline bool addr_has_metadata(const void *addr)
> {
> return (kasan_reset_tag(addr) >=
> kasan_shadow_to_mem((void *)KASAN_SHADOW_START));
> @@ -316,7 +316,7 @@ bool kasan_check_range(unsigned long addr, size_t size, bool write,
>
> #else /* CONFIG_KASAN_GENERIC || CONFIG_KASAN_SW_TAGS */
>
> -static inline bool addr_has_metadata(const void *addr)
> +static __always_inline bool addr_has_metadata(const void *addr)
> {
> return (is_vmalloc_addr(addr) || virt_addr_valid(addr));
> }
> --
> 2.39.1
>
On Wed, Feb 08, 2023 at 05:39:57PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> The memset/memmove/memcpy string functions are wrapped in different
> ways based on configuration. While the __asan_mem* functions already
> have exceptions, the ones called from those do not:
>
> mm/kasan/shadow.o: warning: objtool: __asan_memset+0x30: call to __memset() with UACCESS enabled
> mm/kasan/shadow.o: warning: objtool: __asan_memmove+0x51: call to __memmove() with UACCESS enabled
> mm/kasan/shadow.o: warning: objtool: __asan_memcpy+0x51: call to __memcpy() with UACCESS enabled
> vmlinux.o: warning: objtool: .altinstr_replacement+0x1406: call to memcpy_erms() with UACCESS enabled
> vmlinux.o: warning: objtool: .altinstr_replacement+0xed0: call to memset_erms() with UACCESS enabled
> vmlinux.o: warning: objtool: memset+0x4: call to memset_orig() with UACCESS enabled
> vmlinux.o: warning: objtool: memset+0x4: call to memset_orig() with UACCESS enabled
>
> Add these to the list as well.
>
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> tools/objtool/check.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index 0f67c6a8bc98..e8fb3bf7a2e3 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -1248,6 +1248,13 @@ static const char *uaccess_safe_builtin[] = {
> "clear_user_erms",
> "clear_user_rep_good",
> "clear_user_original",
> + "__memset",
> + "__memcpy",
> + "__memmove",
> + "memset_erms",
> + "memcpy_erms",
> + "memset_orig",
> + "memcpy_orig",
> NULL
> };
Hmm, I wanted to go the other way and remove __asan_mem*.
https://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git/commit/?h=sched/core-robot&id=79cdfdacd5b8d1ac77e24ccbc178bba0294d0d78
On Wed, Feb 08, 2023 at 05:39:55PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> When the compiler decides not to inline this function, objdump
> complains about incorrect UACCESS state:
>
> mm/kasan/generic.o: warning: objtool: __asan_load2+0x11: call to addr_has_metadata() with UACCESS enabled
>
> Signed-off-by: Arnd Bergmann <[email protected]>
Acked-by: Peter Zijlstra (Intel) <[email protected]>
> ---
> mm/kasan/kasan.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
> index 3231314e071f..9377b0789edc 100644
> --- a/mm/kasan/kasan.h
> +++ b/mm/kasan/kasan.h
> @@ -297,7 +297,7 @@ static inline const void *kasan_shadow_to_mem(const void *shadow_addr)
> << KASAN_SHADOW_SCALE_SHIFT);
> }
>
> -static inline bool addr_has_metadata(const void *addr)
> +static __always_inline bool addr_has_metadata(const void *addr)
> {
> return (kasan_reset_tag(addr) >=
> kasan_shadow_to_mem((void *)KASAN_SHADOW_START));
> @@ -316,7 +316,7 @@ bool kasan_check_range(unsigned long addr, size_t size, bool write,
>
> #else /* CONFIG_KASAN_GENERIC || CONFIG_KASAN_SW_TAGS */
>
> -static inline bool addr_has_metadata(const void *addr)
> +static __always_inline bool addr_has_metadata(const void *addr)
> {
> return (is_vmalloc_addr(addr) || virt_addr_valid(addr));
> }
> --
> 2.39.1
>
On Wed, Feb 8, 2023, at 19:09, Peter Zijlstra wrote:
> On Wed, Feb 08, 2023 at 05:39:57PM +0100, Arnd Bergmann wrote:
>
> Hmm, I wanted to go the other way and remove __asan_mem*.
>
>
> https://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git/commit/?h=sched/core-robot&id=79cdfdacd5b8d1ac77e24ccbc178bba0294d0d78
Makes sense. I've put your patch into randconfig tree now, I'll let
you know if that causes other problems.
Arnd
On Wed, Feb 8, 2023, at 18:00, Marco Elver wrote:
>> CFLAGS_REMOVE.o = $(CC_FLAGS_FTRACE)
>
> That means this CFLAGS_REMOVE.o didn't work, right? Can it be removed?
>
Ah, I missed this. Adjusted the patch and description accordingly.
Arnd
On Wed, Feb 8, 2023, at 17:59, Marco Elver wrote:
> On Wed, 8 Feb 2023 at 17:40, Arnd Bergmann <[email protected]> wrote:
>>
>> From: Arnd Bergmann <[email protected]>
>>
>> A lot of the tsan helpers are already excempt from the UACCESS warnings,
>> but some more functions were added that need the same thing:
>>
>> kernel/kcsan/core.o: warning: objtool: __tsan_volatile_read16+0x0: call to __tsan_unaligned_read16() with UACCESS enabled
>> kernel/kcsan/core.o: warning: objtool: __tsan_volatile_write16+0x0: call to __tsan_unaligned_write16() with UACCESS enabled
>> vmlinux.o: warning: objtool: __tsan_unaligned_volatile_read16+0x4: call to __tsan_unaligned_read16() with UACCESS enabled
>> vmlinux.o: warning: objtool: __tsan_unaligned_volatile_write16+0x4: call to __tsan_unaligned_write16() with UACCESS enabled
>
> That's odd - this has never been needed, because all __tsan_unaligned
> are aliases for the non-unaligned functions. And all those are in the
> uaccess_safe_builtin list already.
>
> So if suddenly the alias name becomes the symbol that objtool sees, we
> might need to add all the other functions as well.
>
> Is this a special build with a new compiler?
I see this with gcc-12 and gcc-13 but not with clang-{14,15,16}, have
not tried any older versions recently.
What I see in the .s file for one of the affected configs is
.globl __tsan_unaligned_read16
.set __tsan_unaligned_read16,__tsan_read16
.p2align 6
.globl __tsan_volatile_read16
.type __tsan_volatile_read16, @function
__tsan_volatile_read16:
endbr64
jmp __tsan_read16 #
.size __tsan_volatile_read16, .-__tsan_volatile_read16
.globl __tsan_unaligned_volatile_read16
.set __tsan_unaligned_volatile_read16,__tsan_volatile_read16
...
.set __tsan_unaligned_write16,__tsan_write16
.p2align 6
.globl __tsan_volatile_write16
.type __tsan_volatile_write16, @function
__tsan_volatile_write16:
endbr64
jmp __tsan_write16 #
.size __tsan_volatile_write16, .-__tsan_volatile_write16
.globl __tsan_unaligned_volatile_write16
.set __tsan_unaligned_volatile_write16,__tsan_volatile_write16
In the object file that turns into:
0000000000004e80 <__tsan_unaligned_volatile_read16>:
4e80: f3 0f 1e fa endbr64
4e84: e9 b7 fe ff ff jmp 4d40 <__tsan_read16>
...
0000000000005000 <__tsan_unaligned_volatile_write16>:
5000: f3 0f 1e fa endbr64
5004: e9 b7 fe ff ff jmp 4ec0 <__tsan_unaligned_write16>
It appears like it picks randomly between the original name
and the alias here, no idea why. Using the clang integrated assembler
to build the .o file from the gcc generated .s file shows the same
code as
0000000000004e80 <__tsan_unaligned_volatile_read16>:
4e80: f3 0f 1e fa endbr64
4e84: e9 00 00 00 00 jmp 4e89 <__tsan_unaligned_volatile_read16+0x9>
4e85: R_X86_64_PLT32 __tsan_read16-0x4
...
0000000000005000 <__tsan_unaligned_volatile_write16>:
5000: f3 0f 1e fa endbr64
5004: e9 00 00 00 00 jmp 5009 <__tsan_unaligned_volatile_write16+0x9>
5005: R_X86_64_PLT32 __tsan_write16-0x4
Attaching the object file for reference.
Arnd
On Wed, 8 Feb 2023 at 20:53, Arnd Bergmann <[email protected]> wrote:
>
> On Wed, Feb 8, 2023, at 17:59, Marco Elver wrote:
> > On Wed, 8 Feb 2023 at 17:40, Arnd Bergmann <[email protected]> wrote:
> >>
> >> From: Arnd Bergmann <[email protected]>
> >>
> >> A lot of the tsan helpers are already excempt from the UACCESS warnings,
> >> but some more functions were added that need the same thing:
> >>
> >> kernel/kcsan/core.o: warning: objtool: __tsan_volatile_read16+0x0: call to __tsan_unaligned_read16() with UACCESS enabled
> >> kernel/kcsan/core.o: warning: objtool: __tsan_volatile_write16+0x0: call to __tsan_unaligned_write16() with UACCESS enabled
> >> vmlinux.o: warning: objtool: __tsan_unaligned_volatile_read16+0x4: call to __tsan_unaligned_read16() with UACCESS enabled
> >> vmlinux.o: warning: objtool: __tsan_unaligned_volatile_write16+0x4: call to __tsan_unaligned_write16() with UACCESS enabled
> >
> > That's odd - this has never been needed, because all __tsan_unaligned
> > are aliases for the non-unaligned functions. And all those are in the
> > uaccess_safe_builtin list already.
> >
> > So if suddenly the alias name becomes the symbol that objtool sees, we
> > might need to add all the other functions as well.
> >
> > Is this a special build with a new compiler?
>
> I see this with gcc-12 and gcc-13 but not with clang-{14,15,16}, have
> not tried any older versions recently.
>
> What I see in the .s file for one of the affected configs is
>
> .globl __tsan_unaligned_read16
> .set __tsan_unaligned_read16,__tsan_read16
> .p2align 6
> .globl __tsan_volatile_read16
> .type __tsan_volatile_read16, @function
> __tsan_volatile_read16:
> endbr64
> jmp __tsan_read16 #
> .size __tsan_volatile_read16, .-__tsan_volatile_read16
> .globl __tsan_unaligned_volatile_read16
> .set __tsan_unaligned_volatile_read16,__tsan_volatile_read16
> ...
> .set __tsan_unaligned_write16,__tsan_write16
> .p2align 6
> .globl __tsan_volatile_write16
> .type __tsan_volatile_write16, @function
> __tsan_volatile_write16:
> endbr64
> jmp __tsan_write16 #
> .size __tsan_volatile_write16, .-__tsan_volatile_write16
> .globl __tsan_unaligned_volatile_write16
> .set __tsan_unaligned_volatile_write16,__tsan_volatile_write16
>
>
> In the object file that turns into:
>
> 0000000000004e80 <__tsan_unaligned_volatile_read16>:
> 4e80: f3 0f 1e fa endbr64
> 4e84: e9 b7 fe ff ff jmp 4d40 <__tsan_read16>
> ...
> 0000000000005000 <__tsan_unaligned_volatile_write16>:
> 5000: f3 0f 1e fa endbr64
> 5004: e9 b7 fe ff ff jmp 4ec0 <__tsan_unaligned_write16>
>
>
> It appears like it picks randomly between the original name
> and the alias here, no idea why. Using the clang integrated assembler
> to build the .o file from the gcc generated .s file shows the same
> code as
>
> 0000000000004e80 <__tsan_unaligned_volatile_read16>:
> 4e80: f3 0f 1e fa endbr64
> 4e84: e9 00 00 00 00 jmp 4e89 <__tsan_unaligned_volatile_read16+0x9>
> 4e85: R_X86_64_PLT32 __tsan_read16-0x4
> ...
> 0000000000005000 <__tsan_unaligned_volatile_write16>:
> 5000: f3 0f 1e fa endbr64
> 5004: e9 00 00 00 00 jmp 5009 <__tsan_unaligned_volatile_write16+0x9>
> 5005: R_X86_64_PLT32 __tsan_write16-0x4
Interesting - also note that in kernel/kcsan/core.c, these functions
don't even call each other explicitly. Although because sizeof(long) <
16 everywhere, the code for the volatile and non-volatile 16-byte
variants ends up the same. So the optimizer seems to think it's ok to
just "call" the other equivalent function, even though we didn't tell
it to do so - check_access() is __always_inline.
Whatever happens here isn't completely wrong, so if you just want to
silence the warning:
Acked-by: Marco Elver <[email protected]>
But I have a feeling the compiler is being a bit too clever here.
On Wed, Feb 8, 2023 at 8:32 PM Arnd Bergmann <[email protected]> wrote:
>
> On Wed, Feb 8, 2023, at 18:00, Marco Elver wrote:
>
> >> CFLAGS_REMOVE.o = $(CC_FLAGS_FTRACE)
> >
> > That means this CFLAGS_REMOVE.o didn't work, right? Can it be removed?
> >
>
> Ah, I missed this. Adjusted the patch and description accordingly.
>
> Arnd
Acked-by: Alexander Potapenko <[email protected]>
(assuming you did, b/c I couldn't find the new version)
On Wed, Feb 08, 2023 at 05:39:55PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> When the compiler decides not to inline this function, objdump
"objdump" -> "objtool" here and in patch subject.
--
Josh
On Wed, Feb 8, 2023 at 5:40 PM Arnd Bergmann <[email protected]> wrote:
>
> From: Arnd Bergmann <[email protected]>
>
> When the compiler decides not to inline this function, objdump
> complains about incorrect UACCESS state:
>
> mm/kasan/generic.o: warning: objtool: __asan_load2+0x11: call to addr_has_metadata() with UACCESS enabled
>
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> mm/kasan/kasan.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
> index 3231314e071f..9377b0789edc 100644
> --- a/mm/kasan/kasan.h
> +++ b/mm/kasan/kasan.h
> @@ -297,7 +297,7 @@ static inline const void *kasan_shadow_to_mem(const void *shadow_addr)
> << KASAN_SHADOW_SCALE_SHIFT);
> }
>
> -static inline bool addr_has_metadata(const void *addr)
> +static __always_inline bool addr_has_metadata(const void *addr)
> {
> return (kasan_reset_tag(addr) >=
> kasan_shadow_to_mem((void *)KASAN_SHADOW_START));
> @@ -316,7 +316,7 @@ bool kasan_check_range(unsigned long addr, size_t size, bool write,
>
> #else /* CONFIG_KASAN_GENERIC || CONFIG_KASAN_SW_TAGS */
>
> -static inline bool addr_has_metadata(const void *addr)
> +static __always_inline bool addr_has_metadata(const void *addr)
> {
> return (is_vmalloc_addr(addr) || virt_addr_valid(addr));
> }
> --
> 2.39.1
>
Reviewed-by: Andrey Konovalov <[email protected]>
Thanks!