From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
To: Hugh Dickins <hughd@google.com>, Ingo Molnar <mingo@redhat.com>,
akpm@linux-foundation.org
Cc: tglx@linutronix.de, paskripkin@gmail.com, rostedt@goodmis.org,
glider@google.com, ebiederm@xmission.com,
linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com,
syzbot <syzbot+b1a83ab2a9eb9321fbdd@syzkaller.appspotmail.com>,
Linus Torvalds <torvalds@linux-foundation.org>
Subject: [PATCH (REPOST)] profiling: initialize prof_cpu_mask from profile_online_cpu()
Date: Sat, 27 Apr 2024 15:51:47 +0900 [thread overview]
Message-ID: <239eadaf-d694-4dff-89b9-b476be35f4e9@I-love.SAKURA.ne.jp> (raw)
In-Reply-To: <85edf211-aa30-4671-93e0-5173b3f7adf2@I-love.SAKURA.ne.jp>
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>
---
Can somebody test this patch? I don't know how not using
cpu_possible_mask affects expected profile data collection
(especially when written to /sys/kernel/profiling interface
when some of CPUs are offline).
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 495b69a71a5d..0540483ae7f0 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 2b775cc5c28f..1a6c1cf98485 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;
@@ -438,6 +436,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
next prev parent reply other threads:[~2024-04-27 6:52 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-26 15:59 [syzbot] [kernel?] KMSAN: uninit-value in profile_hits (3) syzbot
2023-12-29 16:37 ` Tetsuo Handa
2023-12-29 16:56 ` syzbot
2024-01-31 14:06 ` [PATCH] profiling: initialize prof_cpu_mask from profile_online_cpu() Tetsuo Handa
2024-02-15 14:41 ` Tetsuo Handa
2024-04-27 6:51 ` Tetsuo Handa [this message]
2024-05-05 6:18 ` [PATCH (REPOST)] " Tetsuo Handa
2023-12-30 5:38 ` [syzbot] Re: [syzbot] [kernel?] KMSAN: uninit-value in profile_hits (3) syzbot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=239eadaf-d694-4dff-89b9-b476be35f4e9@I-love.SAKURA.ne.jp \
--to=penguin-kernel@i-love.sakura.ne.jp \
--cc=akpm@linux-foundation.org \
--cc=ebiederm@xmission.com \
--cc=glider@google.com \
--cc=hughd@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=paskripkin@gmail.com \
--cc=rostedt@goodmis.org \
--cc=syzbot+b1a83ab2a9eb9321fbdd@syzkaller.appspotmail.com \
--cc=syzkaller-bugs@googlegroups.com \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.