Hello,
syzbot found the following issue on:
HEAD commit: 1978a14f70af x86: kmsan: enable KMSAN builds for x86
git tree: https://github.com/google/kmsan.git master
console output: https://syzkaller.appspot.com/x/log.txt?x=13b7a95b700000
kernel config: https://syzkaller.appspot.com/x/.config?x=d830111cc3be873
dashboard link: https://syzkaller.appspot.com/bug?extid=b1a83ab2a9eb9321fbdd
compiler: clang version 14.0.0 (/usr/local/google/src/llvm-git-monorepo 2b554920f11c8b763cd9ed9003f4e19b919b8e1f), GNU ld (GNU Binutils for Debian) 2.35.2
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=14b5476b700000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=142c9237700000
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: [email protected]
=====================================================
BUG: KMSAN: uninit-value in do_profile_hits kernel/profile.c:236 [inline]
BUG: KMSAN: uninit-value in profile_hits+0xaf2/0x1260 kernel/profile.c:326
do_profile_hits kernel/profile.c:236 [inline]
profile_hits+0xaf2/0x1260 kernel/profile.c:326
profile_hit include/linux/profile.h:58 [inline]
profile_tick+0x241/0x250 kernel/profile.c:336
tick_sched_handle kernel/time/tick-sched.c:227 [inline]
tick_sched_timer+0x4bd/0x610 kernel/time/tick-sched.c:1428
__run_hrtimer+0x49f/0xc50 kernel/time/hrtimer.c:1685
__hrtimer_run_queues kernel/time/hrtimer.c:1749 [inline]
hrtimer_interrupt+0x7f7/0x2100 kernel/time/hrtimer.c:1811
local_apic_timer_interrupt arch/x86/kernel/apic/apic.c:1086 [inline]
__sysvec_apic_timer_interrupt+0x178/0x5e0 arch/x86/kernel/apic/apic.c:1103
sysvec_apic_timer_interrupt+0x9d/0xc0 arch/x86/kernel/apic/apic.c:1097
asm_sysvec_apic_timer_interrupt+0x12/0x20
__raw_spin_unlock_irq include/linux/spinlock_api_smp.h:160 [inline]
_raw_spin_unlock_irq+0x36/0x60 kernel/locking/spinlock.c:202
spin_unlock_irq include/linux/spinlock.h:399 [inline]
__set_current_blocked+0xb0c/0xb90 kernel/signal.c:3051
sigprocmask kernel/signal.c:3085 [inline]
__do_sys_rt_sigprocmask kernel/signal.c:3162 [inline]
__se_sys_rt_sigprocmask+0x438/0x5b0 kernel/signal.c:3145
__x64_sys_rt_sigprocmask+0x11e/0x170 kernel/signal.c:3145
do_syscall_x64 arch/x86/entry/common.c:51 [inline]
do_syscall_64+0x54/0xd0 arch/x86/entry/common.c:81
entry_SYSCALL_64_after_hwframe+0x44/0xae
Local variable iter.i created at:
new_sync_read fs/read_write.c:393 [inline]
vfs_read+0xb8a/0x1980 fs/read_write.c:481
ksys_read+0x28b/0x510 fs/read_write.c:619
CPU: 1 PID: 3474 Comm: sshd Not tainted 5.17.0-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
=====================================================
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at [email protected].
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title
If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.
If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)
If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report
If you want to undo deduplication, reply with:
#syz undup
[PATCH] profiling: initialize prof_cpu_mask from profile_online_cpu()
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: [email protected]
Closes: https://syzkaller.appspot.com/bug?extid=b1a83ab2a9eb9321fbdd
Fixes: acd895795d35 ("profiling: fix broken profiling regression")
Signed-off-by: Tetsuo Handa <[email protected]>
---
Please test before applying this patch; I don't know how to test this functionality.
#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
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
Hello,
syzbot has tested the proposed patch and the reproducer did not trigger any issue:
Reported-and-tested-by: [email protected]
Tested on:
commit: 8735c7c8 Merge tag '6.7rc7-smb3-srv-fix' of git://git...
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=122dcf81e80000
kernel config: https://syzkaller.appspot.com/x/.config?x=b2fd2f495c90e6b7
dashboard link: https://syzkaller.appspot.com/bug?extid=b1a83ab2a9eb9321fbdd
compiler: gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
patch: https://syzkaller.appspot.com/x/patch.diff?x=13f021b5e80000
Note: testing is done by a robot and is best-effort only.
For archival purposes, forwarding an incoming command email to
[email protected].
***
Subject: Re: [syzbot] [kernel?] KMSAN: uninit-value in profile_hits (3)
Author: [email protected]
please test uninit-value in profile_hits
#syz test https://github.com/google/kmsan.git master
diff --git a/kernel/profile.c b/kernel/profile.c
index 8a77769bc4b4..37f30292b626 100644
--- a/kernel/profile.c
+++ b/kernel/profile.c
@@ -114,7 +114,7 @@ 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);
Hello,
syzbot has tested the proposed patch and the reproducer did not trigger any issue:
Reported-and-tested-by: [email protected]
Tested on:
commit: d1d7f15c DO-NOT-SUBMIT: kmsan: add the kmsan_exceed_ma..
git tree: https://github.com/google/kmsan.git master
console output: https://syzkaller.appspot.com/x/log.txt?x=14287b09e80000
kernel config: https://syzkaller.appspot.com/x/.config?x=c990527cdcc61224
dashboard link: https://syzkaller.appspot.com/bug?extid=b1a83ab2a9eb9321fbdd
compiler: gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
patch: https://syzkaller.appspot.com/x/patch.diff?x=14033b31e80000
Note: testing is done by a robot and is best-effort only.
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: [email protected]
Closes: https://syzkaller.appspot.com/bug?extid=b1a83ab2a9eb9321fbdd
Fixes: acd895795d35 ("profiling: fix broken profiling regression")
Tested-by: [email protected]
Signed-off-by: Tetsuo Handa <[email protected]>
---
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
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: [email protected]
> Closes: https://syzkaller.appspot.com/bug?extid=b1a83ab2a9eb9321fbdd
> Fixes: acd895795d35 ("profiling: fix broken profiling regression")
> Tested-by: [email protected]
> Signed-off-by: Tetsuo Handa <[email protected]>
> ---
> 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);
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: [email protected]
Closes: https://syzkaller.appspot.com/bug?extid=b1a83ab2a9eb9321fbdd
Fixes: acd895795d35 ("profiling: fix broken profiling regression")
Tested-by: [email protected]
Signed-off-by: Tetsuo Handa <[email protected]>
---
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