linux-kernel - Re: [PATCH v2] acpi/ghes: Prevent sleeping with spinlock held

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]

Message-ID: <65ccf437ec4ee_cc44c29485@iweiny-mobl.notmuch>
Date: Wed, 14 Feb 2024 09:11:19 -0800
From: Ira Weiny <ira.weiny@...el.com>
To: Ira Weiny <ira.weiny@...el.com>, Jonathan Cameron
	<Jonathan.Cameron@...wei.com>
CC: "Rafael J. Wysocki" <rafael@...nel.org>, Dan Williams
	<dan.j.williams@...el.com>, Smita Koralahalli
	<Smita.KoralahalliChannabasappa@....com>, <linux-acpi@...r.kernel.org>,
	<linux-cxl@...r.kernel.org>, <linux-kernel@...r.kernel.org>, Dan Carpenter
	<dan.carpenter@...aro.org>, Steven Rostedt <rostedt@...dmis.org>, "Masami
 Hiramatsu" <mhiramat@...nel.org>, Mathieu Desnoyers
	<mathieu.desnoyers@...icios.com>, <linux-trace-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] acpi/ghes: Prevent sleeping with spinlock held

Ira Weiny wrote:
> Jonathan Cameron wrote:
> > On Tue, 06 Feb 2024 14:15:32 -0800
> > Ira Weiny <ira.weiny@...el.com> wrote:
> > 
> >

[snip]

> > 
> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > index 9ff8a439d674..7ee45f22f56f 100644
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c
> > @@ -2957,7 +2957,7 @@ static void output_printk(struct trace_event_buffer *fbuffer)
> >         iter->ent = fbuffer->entry;
> >         event_call->event.funcs->trace(iter, 0, event);
> >         trace_seq_putc(&iter->seq, 0);
> > -       printk("%s", iter->seq.buffer);
> > +       printk_deferred("%s", iter->seq.buffer);
> > 
> >         raw_spin_unlock_irqrestore(&tracepoint_iter_lock, flags);
> >  }
> > 
> > My assumption is similar views will apply here to Peter Zijlstra's comment on
> > not using printk_deferred.
> > 
> > https://lore.kernel.org/all/20231010141244.GM377@noisy.programming.kicks-ass.net/
> > 
> > Note I also tried the hacks Peter links to from there. They trip issues in the initial
> > CPER print - so I assume not appropriate here.
> > 
> > So I'm thinking this is a won't fix - wait for the printk rework to land and
> > assume this will be resolved as well?
> > 
> 
> Or could we avoid the situation entirely by using a static call?
> 
> The work queue still needs to be created because of the atomicness of
> ghes_do_proc() but it avoids cxl_cper_rw_sem.
> 
> I think the hardest thing may be writing the commit message to explain all
> this.  :-(
> 

Never mind, I already went down that path.  I was surprised I did not
mention it in this commit message.  I did in V1.  :-(

"A static call was considered but ARM does not select HAVE_STATIC_CALL
and in that case setting the function pointer uses a RW semaphore."
	-- https://lore.kernel.org/all/20240202-cxl-cper-smatch-v1-1-7a4103c7f5a0@intel.com/

Should have carried that through.

So should we revert ...

	Fixes: 671a794c33c6 ("acpi/ghes: Process CXL Component Events") 

.. and wait for the printk fix or just move forward with this patch?

Sorry for the noise,
Ira