lkml.org 
[lkml]   [2024]   [Jan]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    Subject[PATCH] profiling: initialize prof_cpu_mask from profile_online_cpu()
    From
    syzbot is reporting uninit-value at profile_hits(), for commit acd895795d35
    ("profiling: fix broken profiling regression") by error initialized
    prof_cpu_mask too early.

    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 the syzbot's report says that profile_hits()
    was called while current thread is still doing vzalloc(buffer_bytes)
    where prof_buffer is NULL at this moment. This indicates two things.

    One is that cpumask_set_cpu(cpu, prof_cpu_mask) should have been called
    from profile_online_cpu() from cpuhp_setup_state() only after
    profile_init() completed. Fix this by explicitly calling cpumask_copy()
    from create_proc_profile() on only UP kernels.

    The other is that multiple threads concurrently tried to write to
    /sys/kernel/profiling interface, which caused that somebody else tried
    to re-initialize prof_buffer despite somebody has already initialized
    prof_buffer. Fix this by using serialization.

    Reported-by: syzbot+b1a83ab2a9eb9321fbdd@syzkaller.appspotmail.com
    Closes: https://syzkaller.appspot.com/bug?extid=b1a83ab2a9eb9321fbdd
    Fixes: acd895795d35 ("profiling: fix broken profiling regression")
    Tested-by: syzbot+b1a83ab2a9eb9321fbdd@syzkaller.appspotmail.com
    Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
    ---
    kernel/ksysfs.c | 27 ++++++++++++++++++++++-----
    kernel/profile.c | 6 +++---
    2 files changed, 25 insertions(+), 8 deletions(-)

    diff --git a/kernel/ksysfs.c b/kernel/ksysfs.c
    index 1d4bc493b2f4..66bc712f590c 100644
    --- a/kernel/ksysfs.c
    +++ b/kernel/ksysfs.c
    @@ -91,10 +91,23 @@ static ssize_t profiling_store(struct kobject *kobj,
    struct kobj_attribute *attr,
    const char *buf, size_t count)
    {
    + static DEFINE_MUTEX(lock);
    int ret;

    - if (prof_on)
    - return -EEXIST;
    + /*
    + * We need serialization, for profile_setup() initializes prof_on
    + * value. Also, use killable wait in case memory allocation from
    + * profile_init() triggered the OOM killer and chose current thread
    + * blocked here.
    + */
    + if (mutex_lock_killable(&lock))
    + return -EINTR;
    +
    + if (prof_on) {
    + count = -EEXIST;
    + goto out;
    + }
    +
    /*
    * This eventually calls into get_option() which
    * has a ton of callers and is not const. It is
    @@ -102,11 +115,15 @@ static ssize_t profiling_store(struct kobject *kobj,
    */
    profile_setup((char *)buf);
    ret = profile_init();
    - if (ret)
    - return ret;
    + if (ret) {
    + count = ret;
    + goto out;
    + }
    ret = create_proc_profile();
    if (ret)
    - return ret;
    + count = ret;
    +out:
    + mutex_unlock(&lock);
    return count;
    }
    KERNEL_ATTR_RW(profiling);
    diff --git a/kernel/profile.c b/kernel/profile.c
    index 8a77769bc4b4..7575747e2ac6 100644
    --- a/kernel/profile.c
    +++ b/kernel/profile.c
    @@ -114,11 +114,9 @@ int __ref profile_init(void)

    buffer_bytes = prof_len*sizeof(atomic_t);

    - if (!alloc_cpumask_var(&prof_cpu_mask, GFP_KERNEL))
    + if (!zalloc_cpumask_var(&prof_cpu_mask, GFP_KERNEL))
    return -ENOMEM;

    - cpumask_copy(prof_cpu_mask, cpu_possible_mask);
    -
    prof_buffer = kzalloc(buffer_bytes, GFP_KERNEL|__GFP_NOWARN);
    if (prof_buffer)
    return 0;
    @@ -481,6 +479,8 @@ int __ref create_proc_profile(void)
    goto err_state_prep;
    online_state = err;
    err = 0;
    +#else
    + cpumask_copy(prof_cpu_mask, cpu_possible_mask);
    #endif
    entry = proc_create("profile", S_IWUSR | S_IRUGO,
    NULL, &profile_proc_ops);
    --
    2.18.4


    \
     
     \ /
      Last update: 2024-05-27 14:43    [W:3.491 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site