[PATCH 1/4] kasan: mark addr_has_metadata __always_inline

[PATCH 1/4] kasan: mark addr_has_metadata __always_inline

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

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 <arnd@arndb.de>
---
 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

[PATCH 2/4] kmsan: disable ftrace in kmsan core code

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

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 <arnd@arndb.de>
---
 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

[PATCH 3/4] objdump: add UACCESS exception for more stringops

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

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 <arnd@arndb.de>
---
 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

[PATCH 4/4] objtool: add UACCESS exceptions for __tsan_volatile_read/write

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

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 <arnd@arndb.de>
---
 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

Re: [PATCH 4/4] objtool: add UACCESS exceptions for __tsan_volatile_read/write

[Marco Elver]

On Wed, 8 Feb 2023 at 17:40, Arnd Bergmann <arnd@kernel.org> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> 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 <arnd@arndb.de>
> ---
>  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 kasan-dev+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20230208164011.2287122-4-arnd%40kernel.org.

Re: [PATCH 2/4] kmsan: disable ftrace in kmsan core code

[Marco Elver]

On Wed, 8 Feb 2023 at 17:40, Arnd Bergmann <arnd@kernel.org> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> 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 <arnd@arndb.de>
> ---
>  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
>

Re: [PATCH 1/4] kasan: mark addr_has_metadata __always_inline

[Marco Elver]

On Wed, 8 Feb 2023 at 17:40, Arnd Bergmann <arnd@kernel.org> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> 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 <arnd@arndb.de>

Reviewed-by: Marco Elver <elver@google.com>

> ---
>  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
>

Re: [PATCH 3/4] objdump: add UACCESS exception for more stringops

[Peter Zijlstra]

On Wed, Feb 08, 2023 at 05:39:57PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> 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 <arnd@arndb.de>
> ---
>  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

Re: [PATCH 1/4] kasan: mark addr_has_metadata __always_inline

[Peter Zijlstra]

On Wed, Feb 08, 2023 at 05:39:55PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> 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 <arnd@arndb.de>

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

> ---
>  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
> 

Re: [PATCH 3/4] objdump: add UACCESS exception for more stringops

[Arnd Bergmann]

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

Re: [PATCH 2/4] kmsan: disable ftrace in kmsan core code

[Arnd Bergmann]

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

Re: [PATCH 4/4] objtool: add UACCESS exceptions for __tsan_volatile_read/write

[Arnd Bergmann]

[-- Attachment #1: Type: text/plain, Size: 3425 bytes --]

On Wed, Feb 8, 2023, at 17:59, Marco Elver wrote:
> On Wed, 8 Feb 2023 at 17:40, Arnd Bergmann <arnd@kernel.org> wrote:
>>
>> From: Arnd Bergmann <arnd@arndb.de>
>>
>> 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

[-- Attachment #2: core.o.gz --]
[-- Type: application/gzip, Size: 21177 bytes --]

Re: [PATCH 4/4] objtool: add UACCESS exceptions for __tsan_volatile_read/write

[Marco Elver]

On Wed, 8 Feb 2023 at 20:53, Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Wed, Feb 8, 2023, at 17:59, Marco Elver wrote:
> > On Wed, 8 Feb 2023 at 17:40, Arnd Bergmann <arnd@kernel.org> wrote:
> >>
> >> From: Arnd Bergmann <arnd@arndb.de>
> >>
> >> 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 <elver@google.com>

But I have a feeling the compiler is being a bit too clever here.

Re: [PATCH 2/4] kmsan: disable ftrace in kmsan core code

[Alexander Potapenko]

On Wed, Feb 8, 2023 at 8:32 PM Arnd Bergmann <arnd@arndb.de> 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 <glider@google.com>

(assuming you did, b/c I couldn't find the new version)

Re: [PATCH 1/4] kasan: mark addr_has_metadata __always_inline

[Josh Poimboeuf]

On Wed, Feb 08, 2023 at 05:39:55PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> When the compiler decides not to inline this function, objdump

"objdump" -> "objtool" here and in patch subject.

-- 
Josh

Re: [PATCH 1/4] kasan: mark addr_has_metadata __always_inline

[Andrey Konovalov]

On Wed, Feb 8, 2023 at 5:40 PM Arnd Bergmann <arnd@kernel.org> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> 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 <arnd@arndb.de>
> ---
>  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 <andreyknvl@gmail.com>

Thanks!