lkml.org 
[lkml]   [2021]   [Dec]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 1/3] X86/db: Change __this_cpu_read() to this_cpu_read() in hw_breakpoint_active()
    Date
    From: Lai Jiangshan <laijs@linux.alibaba.com>

    __this_cpu_read() can not be instrumented except its own debugging code
    when CONFIG_DEBUG_PREEMPT. The debugging code will call
    __this_cpu_preempt_check(). __this_cpu_preempt_check() itself is also
    noinstr, so __this_cpu_read() can be used in noinstr.

    But these is one exception when exc_debug_kernel() calls local_db_save()
    which calls hw_breakpoint_active() which calls __this_cpu_read(). If
    the data accessed by __this_cpu_preempt_check() is also watched by
    hw_breakpoints, it would cause recursive #DB.

    this_cpu_read() in X86 is also non instrumentable, and it doesn't access
    to any extra data except the percpu cpu_dr7, and cpu_dr7 is disallowed
    to be watched in arch_build_bp_info(). So this_cpu_read() is safe to
    be used when hw_breakpoints is still active, and __this_cpu_read() here
    should be changed to this_cpu_read().

    This problem can only happen when the system owner uses a kernel with
    CONFIG_DEBUG_PREEMPT enabled and deliberately use hw_breakpoints on
    the data that __this_cpu_preempt_check() accesses. Sot it is just a
    problem with no significance.

    One might suggest that, all the data accessed by noinstr functions
    should be marked in denylist for hw_breakpoints. That would complexify
    the noinstrment framework and add hurdles to anyone that who want to
    add a new noinstr function. All we need is to suppress #DB in the IST
    interrupt entry path until safe place where #DB is disabled in hardware
    or #DB handler can handle well even it hits data accessed by noinstr
    function. Changing __this_cpu_read() to this_cpu_read() is fit for it.

    Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com>
    ---
    arch/x86/include/asm/debugreg.h | 2 +-
    1 file changed, 1 insertion(+), 1 deletion(-)

    diff --git a/arch/x86/include/asm/debugreg.h b/arch/x86/include/asm/debugreg.h
    index cfdf307ddc01..20189ce41578 100644
    --- a/arch/x86/include/asm/debugreg.h
    +++ b/arch/x86/include/asm/debugreg.h
    @@ -87,7 +87,7 @@ static inline void hw_breakpoint_disable(void)

    static __always_inline bool hw_breakpoint_active(void)
    {
    - return __this_cpu_read(cpu_dr7) & DR_GLOBAL_ENABLE_MASK;
    + return this_cpu_read(cpu_dr7) & DR_GLOBAL_ENABLE_MASK;
    }

    extern void hw_breakpoint_restore(void);
    --
    2.19.1.6.gb485710b
    \
     
     \ /
      Last update: 2021-12-13 05:23    [W:2.476 / U:0.080 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site