• Tetsuo Handa's avatar
    profiling: remove prof_cpu_mask · 7c51f7bb
    Tetsuo Handa authored
    syzbot is reporting uninit-value at profile_hits(), for there is a race
    window between
    
      if (!alloc_cpumask_var(&prof_cpu_mask, GFP_KERNEL))
        return -ENOMEM;
      cpumask_copy(prof_cpu_mask, cpu_possible_mask);
    
    in profile_init() and
    
      cpumask_available(prof_cpu_mask) &&
      cpumask_test_cpu(smp_processor_id(), prof_cpu_mask))
    
    in profile_tick(); prof_cpu_mask remains uninitialzed until cpumask_copy()
    completes while cpumask_available(prof_cpu_mask) returns true as soon as
    alloc_cpumask_var(&prof_cpu_mask) completes.
    
    We could replace alloc_cpumask_var() with zalloc_cpumask_var() and
    call cpumask_copy() from create_proc_profile() on only UP kernels, for
    profile_online_cpu() calls cpumask_set_cpu() as needed via
    cpuhp_setup_state(CPUHP_AP_ONLINE_DYN) on SMP kernels. But this patch
    removes prof_cpu_mask because it seems unnecessary.
    
    The cpumask_test_cpu(smp_processor_id(), prof_cpu_mask) test
    in profile_tick() is likely always true due to
    
      a CPU cannot call profile_tick() if that CPU is offline
    
    and
    
      cpumask_set_cpu(cpu, prof_cpu_mask) is called when that CPU becomes
      online and cpumask_clear_cpu(cpu, prof_cpu_mask) is called when that
      CPU becomes offline
    
    . This test could be false during transition between online and offline.
    
    But according to include/linux/cpuhotplug.h , CPUHP_PROFILE_PREPARE
    belongs to PREPARE section, which means that the CPU subjected to
    profile_dead_cpu() cannot be inside profile_tick() (i.e. no risk of
    use-after-free bug) because interrupt for that CPU is disabled during
    PREPARE section. Therefore, this test is guaranteed to be true, and
    can be removed. (Since profile_hits() checks prof_buffer != NULL, we
    don't need to check prof_buffer != NULL here unless get_irq_regs() or
    user_mode() is such slow that we want to avoid when prof_buffer == NULL).
    
    do_profile_hits() is called from profile_tick() from timer interrupt
    only if cpumask_test_cpu(smp_processor_id(), prof_cpu_mask) is true and
    prof_buffer is not NULL. But syzbot is also reporting that sometimes
    do_profile_hits() is called while current thread is still doing vzalloc(),
    where prof_buffer must be NULL at this moment. This indicates that multiple
    threads concurrently tried to write to /sys/kernel/profiling interface,
    which caused that somebody else try to re-allocate prof_buffer despite
    somebody has already allocated prof_buffer. Fix this by using
    serialization.
    Reported-by: default avatarsyzbot <syzbot+b1a83ab2a9eb9321fbdd@syzkaller.appspotmail.com>
    Closes: https://syzkaller.appspot.com/bug?extid=b1a83ab2a9eb9321fbddSigned-off-by: default avatarTetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
    Tested-by: default avatarsyzbot <syzbot+b1a83ab2a9eb9321fbdd@syzkaller.appspotmail.com>
    Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
    7c51f7bb
ksysfs.c 7.64 KB