Re: [PATCH v2 13/33] kmsan: Introduce memset_no_sanitize_memory()
From: Alexander Potapenko <glider@google.com>
Date: 2023-12-08 15:26:28
Also in:
linux-mm, linux-s390, lkml
A problem with __memset() is that, at least for me, it always ends up being a call. There is a use case where we need to write only 1 byte, so I thought that introducing a call there (when compiling without KMSAN) would be unacceptable.
Wonder what happens with that use case if we e.g. build with fortify-source. Calling memset() for a single byte might be indicating the code is not hot.
quoted
...quoted
+__no_sanitize_memory +static inline void *memset_no_sanitize_memory(void *s, int c, size_t n) +{ + return memset(s, c, n); +}I think depending on the compiler optimizations this might end up being a call to normal memset, that would still change the shadow bytes.Interesting, do you have some specific scenario in mind? I vaguely remember that in the past there were cases when sanitizer annotations were lost after inlining, but I thought they were sorted out?
Sanitizer annotations are indeed lost after inlining, and we cannot do much about that. They are implemented using function attributes, and if a function dissolves after inlining, we cannot possibly know which instructions belonged to it. Consider the following example (also available at https://godbolt.org/z/5r7817G8e): ================================== void *kmalloc(int size); __attribute__((no_sanitize("kernel-memory"))) __attribute__((always_inline)) static void *memset_nosanitize(void *s, int c, int n) { return __builtin_memset(s, c, n); } void *do_something_nosanitize(int size) { void *ptr = kmalloc(size); memset_nosanitize(ptr, 0, size); return ptr; } void *do_something_sanitize(int size) { void *ptr = kmalloc(size); __builtin_memset(ptr, 0, size); return ptr; } ================================== If memset_nosanitize() has __attribute__((always_inline)), the compiler generates the same LLVM IR calling __msan_memset() for both do_something_nosanitize() and do_something_sanitize(). If we comment out this attribute, do_something_nosanitize() calls memset_nosanitize(), which doesn't have the sanitize_memory attribute. But even now __builtin_memset() is still calling __msan_memset(), because __attribute__((no_sanitize("kernel-memory"))) somewhat counterintuitively still preserves some instrumentation (see include/linux/compiler-clang.h for details). Replacing __attribute__((no_sanitize("kernel-memory"))) with __attribute__((disable_sanitizer_instrumentation)) fixes this situation: define internal fastcc noundef ptr @memset_nosanitize(void*, int, int)(ptr noundef returned writeonly %s, i32 noundef %n) unnamed_addr #2 { entry: %conv = sext i32 %n to i64 tail call void @llvm.memset.p0.i64(ptr align 1 %s, i8 0, i64 %conv, i1 false) ret ptr %s }
And, in any case, if this were to happen, would not it be considered a compiler bug that needs fixing there, and not in the kernel?
As stated above, I don't think this is more or less working as intended. If we really want the ability to inline __memset(), we could transform it into memset() in non-sanitizer builds, but perhaps having a call is also acceptable?