Re: [PATCH] mm/kmemleak: Fix sleeping function called from invalid context in kmemleak_seq_show

[Alessandro Carminati <acarmina@redhat.com>]

Hi Sebastian,

On Thu, Nov 21, 2024 at 6:04 PM Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
>
> On 2024-11-21 17:50:06 [+0100], Alessandro Carminati wrote:
> > Hello Sebastian,
> Hi Alessandro,
>
> > On Wed, Nov 20, 2024 at 5:40 PM Sebastian Andrzej Siewior
> > <bigeasy@linutronix.de> wrote:
> > >
> > > On 2024-11-20 10:26:02 [-0500], Steven Rostedt wrote:
> > > > The "%pK" dereferences a pointer and there's some SELinux hooks attached to
> > > > that code. The problem is that the SELinux hooks take spinlocks. This would
> > > > not have been an issue if it wasn't for that "%pK" in the format.
> > >
> > > That is missing check and I think Thomas Weissschuh wanted to add it. So
> > > we don't call into selinux.
> >
> > Your comment confuses me a bit, as I'm unsure what Thomas is actually
> > working on.
> > Am I correct in assuming he's addressing a fix in lib/vsprintf.c to ensure
> > that sleeping functions aren't called, allowing these functions to work in
> > any context?
>
> restricted_pointer() has a check for in_hardirq() among others. This
> needs an additional PREEMPT_RT check.
> I would be actual in favour to get rid of case 1 for kptr_restrict and
> have either 0 or 2.
>
> > However, his mention of "This fix for kmemleak is still needed as the
> > pointers in the kmemleak report are useful" adds to my confusion.
> > Meanwhile, Steven suggests reworking SELinux to resolve the issue.
> > Could you clarify what you mean by "So we don't call into selinux"?
>
> This getting out of hand. By adding the PREEMPT_RT check to
> restricted_pointer() we don't call in selinux so the problem is gone.

I am really glad that now we have a clear solution, however practically
speaking is Thomas working on such a patch or is he working on something
related that does not fully solve the problem?

Even if he is working on a partial solution, I am happy to coordinate
off-list working on his own private branch
(or else I would just give up and review the Thomas' patchset when it is
out...)

> kmemleak is not the only problem. printk(), as another of vspritf pointer
> code user, can be called from any place and would also trigger a
> warning here.
> As far as "kmemleak need to be usefull" goes: With kptr_restrict == 0
> then with or without pointer hashing they will be useful. If we need to
> go via selinux then it ends as a hint.
>
> Sebastian
>


-- 
---
172