• Nadav Amit's avatar
    x86/alternative: Fix race in try_get_desc() · efd608fa
    Nadav Amit authored
    I encountered some occasional crashes of poke_int3_handler() when
    kprobes are set, while accessing desc->vec.
    
    The text poke mechanism claims to have an RCU-like behavior, but it
    does not appear that there is any quiescent state to ensure that
    nobody holds reference to desc. As a result, the following race
    appears to be possible, which can lead to memory corruption.
    
      CPU0					CPU1
      ----					----
      text_poke_bp_batch()
      -> smp_store_release(&bp_desc, &desc)
    
      [ notice that desc is on
        the stack			]
    
    					poke_int3_handler()
    
    					[ int3 might be kprobe's
    					  so sync events are do not
    					  help ]
    
    					-> try_get_desc(descp=&bp_desc)
    					   desc = __READ_ONCE(bp_desc)
    
    					   if (!desc) [false, success]
      WRITE_ONCE(bp_desc, NULL);
      atomic_dec_and_test(&desc.refs)
    
      [ success, desc space on the stack
        is being reused and might have
        non-zero value. ]
    					arch_atomic_inc_not_zero(&desc->refs)
    
    					[ might succeed since desc points to
    					  stack memory that was freed and might
    					  be reused. ]
    
    Fix this issue with small backportable patch. Instead of trying to
    make RCU-like behavior for bp_desc, just eliminate the unnecessary
    level of indirection of bp_desc, and hold the whole descriptor as a
    global.  Anyhow, there is only a single descriptor at any given
    moment.
    
    Fixes: 1f676247 ("x86/alternatives: Implement a better poke_int3_handler() completion scheme")
    Signed-off-by: default avatarNadav Amit <namit@vmware.com>
    Signed-off-by: default avatarPeter Zijlstra (Intel) <peterz@infradead.org>
    Cc: stable@kernel.org
    Link: https://lkml.kernel.org/r/20220920224743.3089-1-namit@vmware.com
    efd608fa
alternative.c 41.7 KB