On Tue, Nov 26, 2024 at 09:11:31AM +0100, Thomas Weissschuh wrote: > On Fri, Nov 22, 2024 at 11:48:04AM +0100, Alessandro Carminati wrote: > > On Thu, Nov 21, 2024 at 6:04 PM Sebastian Andrzej Siewior > > <bigeasy@xxxxxxxxxxxxx> wrote: > > > On 2024-11-21 17:50:06 [+0100], Alessandro Carminati wrote: > > > > On Wed, Nov 20, 2024 at 5:40 PM Sebastian Andrzej Siewior > > > > <bigeasy@xxxxxxxxxxxxx> 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? > > I am working on a change to lib/vsprintf.c. Something like the > following: > > diff --git a/lib/vsprintf.c b/lib/vsprintf.c > index c5e2ec9303c5..54ad175a22bd 100644 > --- a/lib/vsprintf.c > +++ b/lib/vsprintf.c > @@ -870,6 +870,9 @@ char *restricted_pointer(char *buf, char *end, const void *ptr, > return error_string(buf, end, "pK-error", spec); > } > > + if (IS_ENABLED(CONFIG_PREEMPT_RT) && ...) > + return error_string(buf, end, "pK-error", spec); > + > /* > * Only print the real pointer value if the current > * process has CAP_SYSLOG and is running with the > > > Hoping to replace the ... with something that can detect actual > problematic situations at runtime. > I'm fairly new to PREEMPT_RT so need to do some digging. > > > 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...) > > Given that there is no direct interaction between your kmemleak patches > and the vsprintf ones I don't think coordination is necessary. > I'll Cc you for review. > > Please do go ahead with your kmemleak patches. If the above fixes the kmemleak warnings by not triggering a call into SELinux, I'd rather not change kmemleak. I don't know how important it is to run kmemleak with kptr_restrict=1 and get meaningful pointers. -- Catalin
