| Message ID | 20231121220155.1217090-14-iii@linux.ibm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | kmsan: Enable on s390 | expand |
On Tue, Nov 21, 2023 at 11:06 PM Ilya Leoshkevich <iii@linux.ibm.com> wrote: > > Add a wrapper for memset() that prevents unpoisoning. We have __memset() already, won't it work for this case? On the other hand, I am not sure you want to preserve the redzone in its previous state (unless it's known to be poisoned). You might consider explicitly unpoisoning the redzone instead. ... > +__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.
> 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. > > ... > > > > > +__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?
diff --git a/include/linux/kmsan.h b/include/linux/kmsan.h index ff8fd95733fa..439df72c8dc6 100644 --- a/include/linux/kmsan.h +++ b/include/linux/kmsan.h @@ -345,4 +345,13 @@ static inline void *kmsan_get_metadata(void *addr, bool is_origin) #endif +/** + * memset_no_sanitize_memory() - memset() without the KMSAN instrumentation. + */ +__no_sanitize_memory +static inline void *memset_no_sanitize_memory(void *s, int c, size_t n) +{ + return memset(s, c, n); +} + #endif /* _LINUX_KMSAN_H */
Add a wrapper for memset() that prevents unpoisoning. This is useful for filling memory allocator redzones. Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> --- include/linux/kmsan.h | 9 +++++++++ 1 file changed, 9 insertions(+)