linux-kernel - Re: [GIT PULL] tracing: Fixes for v6.17

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: <20250910162106.6624752c@gandalf.local.home>
Date: Wed, 10 Sep 2025 16:21:06 -0400
From: Steven Rostedt <rostedt@...dmis.org>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Catalin Marinas <catalin.marinas@....com>, Will Deacon
 <will@...nel.org>, LKML <linux-kernel@...r.kernel.org>, Masami Hiramatsu
 <mhiramat@...nel.org>, Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
 Guenter Roeck <linux@...ck-us.net>, Luo Gengkun
 <luogengkun@...weicloud.com>, Pu Lehui <pulehui@...wei.com>, Qianfeng Rong
 <rongqianfeng@...o.com>, Vladimir Riabchun <ferr.lambarginio@...il.com>,
 Wang Liang <wangliang74@...wei.com>
Subject: Re: [GIT PULL] tracing: Fixes for v6.17

On Wed, 10 Sep 2025 12:37:50 -0700
Linus Torvalds <torvalds@...ux-foundation.org> wrote:

> On Wed, 10 Sept 2025 at 12:19, Linus Torvalds
> <torvalds@...ux-foundation.org> wrote:
> >
> > In this case, the appropriate coding is to typically surround it with
> > a pagefault_{disable,enable}() pattern to let the page faulting code
> > know to not actually do the fault.  
> 
> Btw, I say "typically", because you don't have to do that. The page
> fault code uses
> 
>         if (faulthandler_disabled() ..)
> 
> to decide if it should handle the fault or not, and that checks not
> just if page faults are explicitly disabled, but also checks -
> surprise surprise - "in_atomic()".
> 
> So just being in an explicitly atomic context automatically means that
> __copy_from_user_inatomic() is atomic.
> 
> Which makes me wonder if there is something entirely wrong.
> 
> Because the explanation for this in commit 3d62ab32df06 ("tracing: Fix
> tracing_marker may trigger page fault during preempt_disable") talks
> about the task being preempted in between the
> 
>   ring_buffer_lock_reserve
>   ring_buffer_unlock_commit
> 
> and it sounds like maybe the tracing code isn't disabling preemption
> for the whole sequence?
> 
> Because "in_atomic()" does check the preempt count, and so just being
> non-preemptible should already have disabled page faults.
> 
> Maybe the page fault just ends up being expensive enough that it
> exposes preemption being more *likely* just because the window now is
> much wider.
> 
> Alternatively, this is perhaps an arm64-specific bug where the page
> fault disabling doesn't honor the preemption disable of
> faulthandler_disabled()?
> 
> I did *not* go through the whole arm64 page faulting code: that commit
> talks about do_mem_abort() which is done as part of the common arm64
> fault handling, and if that then doesn't honor
> faulthandler_disabled(), then honestly, that perf fix isn't actually
> fixing anything either. It would still do the same reschedule even
> with an explicit "pagefault_disable()/enable()" if
> faulthandler_disabled() simply isn't honored properly.
> 
> Adding Catalin and Will to the participants to see if they have input
> on that whole do_mem_abort() angle.
> 

We figured out in the discussion that the user that triggered this had
CONFIG_PREEMPT_NONE, where in_atomic() always returns false.

-- Steve