Messages in this thread | | | Date | Thu, 15 Feb 2024 23:41:59 +0900 | Subject | Re: [PATCH] profiling: initialize prof_cpu_mask from profile_online_cpu() | From | Tetsuo Handa <> |
| |
Hello, Linus.
It seems that nobody maintains this code.
Can I directly send this patch to you, as "THE REST" rule of MAINTAINERS file? Is some tag like [PATCH ORPHANED] available?
On 2024/01/31 23:06, Tetsuo Handa wrote: > 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);
| |