linux-kernel - Re: [PATCH] tracing: Fix tracing_marker may trigger page fault during preempt

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <436e4fa7-f8c7-4c23-a28a-4e5eebe2f854@huaweicloud.com>
Date: Fri, 29 Aug 2025 16:29:07 +0800
From: Luo Gengkun <luogengkun@...weicloud.com>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: mhiramat@...nel.org, mathieu.desnoyers@...icios.com,
 linux-kernel@...r.kernel.org, linux-trace-kernel@...r.kernel.org
Subject: Re: [PATCH] tracing: Fix tracing_marker may trigger page fault during
 preempt_disable


On 2025/8/20 1:50, Steven Rostedt wrote:
> On Tue, 19 Aug 2025 10:51:52 +0000
> Luo Gengkun <luogengkun@...weicloud.com> wrote:
>
>> Both tracing_mark_write and tracing_mark_raw_write call
>> __copy_from_user_inatomic during preempt_disable. But in some case,
>> __copy_from_user_inatomic may trigger page fault, and will call schedule()
>> subtly. And if a task is migrated to other cpu, the following warning will
> Wait! What?
>
> __copy_from_user_inatomic() is allowed to be called from in atomic context.
> Hence the name it has. How the hell can it sleep? If it does, it's totally
> broken!
>
> Now, I'm not against using nofault() as it is better named, but I want to
> know why you are suggesting this change. Did you actually trigger a bug here?

yes, I trigger this bug in arm64.

>
>> be trigger:
>>          if (RB_WARN_ON(cpu_buffer,
>>                         !local_read(&cpu_buffer->committing)))
>>
>> An example can illustrate this issue:
>>
>> process flow						CPU
>> ---------------------------------------------------------------------
>>
>> tracing_mark_raw_write():				cpu:0
>>     ...
>>     ring_buffer_lock_reserve():				cpu:0
>>        ...
>>        cpu = raw_smp_processor_id()			cpu:0
>>        cpu_buffer = buffer->buffers[cpu]			cpu:0
>>        ...
>>     ...
>>     __copy_from_user_inatomic():				cpu:0
>>        ...
>>        # page fault
>>        do_mem_abort():					cpu:0
> Sounds to me that arm64 __copy_from_user_inatomic() may be broken.
>
>>           ...
>>           # Call schedule
>>           schedule()					cpu:0
>> 	 ...
>>     # the task schedule to cpu1
>>     __buffer_unlock_commit():				cpu:1
>>        ...
>>        ring_buffer_unlock_commit():			cpu:1
>> 	 ...
>> 	 cpu = raw_smp_processor_id()			cpu:1
>> 	 cpu_buffer = buffer->buffers[cpu]		cpu:1
>>
>> As shown above, the process will acquire cpuid twice and the return values
>> are not the same.
>>
>> To fix this problem using copy_from_user_nofault instead of
>> __copy_from_user_inatomic, as the former performs 'access_ok' before
>> copying.
>>
>> Fixes: 656c7f0d2d2b ("tracing: Replace kmap with copy_from_user() in trace_marker writing")
> The above commit was intorduced in 2016. copy_from_user_nofault() was
> introduced in 2020. I don't think this would be the fix for that kernel.
>
> So no, I'm not taking this patch. If you see __copy_from_user_inatomic()
> sleeping, it's users are not the issue. That function is.
>
> -- Steve
>
>
I noticed that in most places where __copy_from_user_inatomic() is used,
it is within the pagefault_disable/enable() section. When pagefault_disable()
is called, user access methods will no sleep. So I'm going to send a v2patch which use pagefault_disable/enable()to fix this problem. -- Gengkun

>
>> Signed-off-by: Luo Gengkun <luogengkun@...weicloud.com>