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

[Alexander Potapenko <glider@xxxxxxxxxx>]

On Fri, Oct 21, 2022 at 8:19 AM youling 257 <youling257@xxxxxxxxx> 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