linux-kernel - Re: [PATCH v2] mm/kmemleak: Fix sleeping function called from invalid context at print message

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] [day] [month] [year] [list]

Message-ID: <20241218161751.gYbwFkzd@linutronix.de>
Date: Wed, 18 Dec 2024 17:17:51 +0100
From: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To: Alessandro Carminati <acarmina@...hat.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
	Catalin Marinas <catalin.marinas@....com>,
	Clark Williams <clrkwllms@...nel.org>,
	Steven Rostedt <rostedt@...dmis.org>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org, linux-rt-devel@...ts.linux.dev,
	Alessandro Carminati <alessandro.carminati@...il.com>,
	Thomas Weissschuh <thomas.weissschuh@...utronix.de>,
	Juri Lelli <juri.lelli@...hat.com>,
	Gabriele Paoloni <gpaoloni@...hat.com>,
	Eric Chanudet <echanude@...hat.com>, clement.leger@...tlin.com
Subject: Re: [PATCH v2] mm/kmemleak: Fix sleeping function called from
 invalid context at print message

On 2024-12-18 09:38:05 [+0100], Alessandro Carminati wrote:
> On Tue, Dec 17, 2024 at 6:55 PM Andrew Morton <akpm@...ux-foundation.org> wrote:
> >
> > On Tue, 17 Dec 2024 14:20:33 +0000 Alessandro Carminati <acarmina@...hat.com> wrote:
> >
> > > Address a bug in the kernel that triggers a "sleeping function called from
> > > invalid context" warning when /sys/kernel/debug/kmemleak is printed under
> > > specific conditions:
> > > - CONFIG_PREEMPT_RT=y
> > > - Set SELinux as the LSM for the system
> > > - Set kptr_restrict to 1
> > > - kmemleak buffer contains at least one item
> > >
> > > BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:48
> > > in_atomic(): 1, irqs_disabled(): 1, non_block: 0, pid: 136, name: cat
> >
> > -rt is a bit annoying this way.  Things which we expect to work OK are
> > no longer doing so.

Yeah. But lockdep should have complained here earlier. The RT just
enforces that part.

> > > --- a/mm/kmemleak.c
> > > +++ b/mm/kmemleak.c
> > > @@ -373,7 +373,7 @@ static void print_unreferenced(struct seq_file *seq,
> > >
> > >       for (i = 0; i < nr_entries; i++) {
> > >               void *ptr = (void *)entries[i];
> > > -             warn_or_seq_printf(seq, "    [<%pK>] %pS\n", ptr, ptr);
> > > +             warn_or_seq_printf(seq, "    %pS\n", ptr);
> > >       }
> > >  }
> >
> > Before 3a6f33d86baa we were still printing the address, with plain old
> > %p.  Should we restore that, or is %p always useless?
> >
> 
> The intent of 3a6f33d86baa8 is to make pointers available for debugging
> purposes.
> My argument for this change is that %pS provides already sufficient
> information for debugging purposes.
> 
> We essentially have two scenarios concerning %pS:
> 1. When the symbol can be resolved by kallsyms: In this case, a
>    symbol+offset style pointer is emitted, which I find more useful than
>    just the address.

indeed. With the relocation the information is usually hard to make
sense.

> 2. When the symbol cannot be resolved by kallsyms: Here, the raw pointer
>    is emitted. This undermines the original intent of %pK, as both the
>    masked address and the plain address are exposed.
> 
> If we want to be picky, there's an additional factor to consider: KASLR.
> %pS emits its output accounting for randomization, but in most cases, the
> randomized offset information is lost.
> The exception is when the symbol cannot be resolved by kallsyms, though
> this should be rare. It's still worth mentioning.
> 
> Back to your question: %p emits an hashed address that is not
> reversible by design.
> Therefore, in my opinion, it is not particularly useful in a debug
> message.

There is still %px if you want your debug message to show the raw
pointer regardless of pointer hashing. That is something that you could
disable. The %pK format is something that was meant to be used files
like proc or sysfs. With the restrictions you already have on dmesg it
does not seem necessary to have yet another check.

Thank you the patch.

Acked-by: Sebastian Andrzej Siewior <bigeasy@...utronix.de>

Sebastian