linux-kernel - Re: [PATCH 2/3] lib/Kconfig.debug: disable LOCK_DEBUGGING

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]

Date: Tue, 25 Jun 2024 12:06:52 -0700
From: "Paul E. McKenney" <paulmck@...nel.org>
To: Boqun Feng <boqun.feng@...il.com>
Cc: Dave Hansen <dave.hansen@...el.com>,
	Alexander Potapenko <glider@...gle.com>, elver@...gle.com,
	dvyukov@...gle.com, dave.hansen@...ux.intel.com,
	peterz@...radead.org, akpm@...ux-foundation.org, x86@...nel.org,
	linux-kernel@...r.kernel.org, kasan-dev@...glegroups.com,
	"Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>,
	Ingo Molnar <mingo@...hat.com>, Will Deacon <will@...nel.org>,
	Waiman Long <longman@...hat.com>
Subject: Re: [PATCH 2/3] lib/Kconfig.debug: disable LOCK_DEBUGGING_SUPPORT
 under KMSAN

On Tue, Jun 25, 2024 at 11:51:23AM -0700, Boqun Feng wrote:
> On Fri, Jun 21, 2024 at 09:23:25AM -0700, Dave Hansen wrote:
> > On 6/21/24 02:49, Alexander Potapenko wrote:
> > >  config LOCK_DEBUGGING_SUPPORT
> > >  	bool
> > > -	depends on TRACE_IRQFLAGS_SUPPORT && STACKTRACE_SUPPORT && LOCKDEP_SUPPORT
> > > +	depends on TRACE_IRQFLAGS_SUPPORT && STACKTRACE_SUPPORT && LOCKDEP_SUPPORT && !KMSAN
> > >  	default y
> > 
> > This kinda stinks.  Practically, it'll mean that anyone turning on KMSAN
> > will accidentally turn off lockdep.  That's really nasty, especially for
> > folks who are turning on debug options left and right to track down
> > nasty bugs.
> > 
> > I'd *MUCH* rather hide KMSAN:
> > 
> > config KMSAN
> >         bool "KMSAN: detector of uninitialized values use"
> >         depends on HAVE_ARCH_KMSAN && HAVE_KMSAN_COMPILER
> >         depends on DEBUG_KERNEL && !KASAN && !KCSAN
> >         depends on !PREEMPT_RT
> > +	depends on !LOCKDEP
> > 
> > Because, frankly, lockdep is way more important than KMSAN.
> > 
> > But ideally, we'd allow them to coexist somehow.  Have we even discussed
> > the problem with the lockdep folks?  For instance, I'd much rather have
> > a relaxed lockdep with no checking in pfn_valid() than no lockdep at all.
> 
> The only locks used in pfn_valid() are rcu_read_lock_sched(), right? If
> so, could you try (don't tell Paul ;-)) replace rcu_read_lock_sched()
> with preempt_disable() and rcu_read_unlock_sched() with
> preempt_enable()? That would avoid calling into lockdep. If that works
> for KMSAN, we can either have a special rcu_read_lock_sched() or call
> lockdep_recursion_inc() in instrumented pfn_valid() to disable lockdep
> temporarily.
> 
> [Cc Paul]

Don't tell me what?  ;-)

An alternative is to use rcu_read_lock_sched_notrace() and
rcu_read_unlock_sched_notrace().  If you really want to use
preempt_disable() and preempt_enable() instead, you will likely want
the _notrace() variants.

							Thanx, Paul