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

[Sebastian Andrzej Siewior <bigeasy@linutronix.de>]

On 2024-11-21 19:19:05 [+0000], Catalin Marinas wrote:
…
> > Maybe SELinux locks should be converted to raw? I don't know how long that
> > lock is held. There are some loops though :-/
> > 
> > avc_insert():
> > 
> > 	spin_lock_irqsave(lock, flag);
> > 	hlist_for_each_entry(pos, head, list) {
> > 		if (pos->ae.ssid == ssid &&
> > 			pos->ae.tsid == tsid &&
> > 			pos->ae.tclass == tclass) {
> > 			avc_node_replace(node, pos);
> > 			goto found;
> > 		}
> > 	}
> > 	hlist_add_head_rcu(&node->list, head);
> > found:
> > 	spin_unlock_irqrestore(lock, flag);
> > 
> > Perhaps that could be converted to simple RCU?
> > 
> > As I'm sure there's other places that call vsprintf() under a raw_spin_lock
> > or non-preemptable context, perhaps this should be the fix we do.
> 
> My preference would also be to convert SELinux rather than avoiding the
> issue in kmemleak (and other similar places).

No. kmemleak has been made use a raw_spinlock_t because most of what it
does is something that is not used in production on a PREEMPT_RT system
and falls in the same category as lockdep for instance. And that code
calls into LSM/ selinux.
Before making the lock in selinux a raw_spinlock_t you have to think
about the consequences in general and audit the code. From a quick
look, there is also avc_insert() invoked in that callchain which
allocates memory and this is a no no.
Also, if you make the solution here in selinux to use a raw_spinlock_t
you would have to do it also in every LSM as they might be used instead
of selinux.

Therefore, I still prefer adding PREEMPT_RT to the restricted_pointer()
category for atomic invocations.

Sebastian