From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.linutronix.de (146.0.238.70:993) by crypto-ml.lab.linutronix.de with IMAP4-SSL for ; 22 Jan 2019 07:33:46 -0000 Received: from mail.kernel.org ([198.145.29.99]) by Galois.linutronix.de with esmtps (TLS1.2:DHE_RSA_AES_256_CBC_SHA256:256) (Exim 4.80) (envelope-from ) id 1glqZ7-0006Ue-Ad for speck@linutronix.de; Tue, 22 Jan 2019 08:33:45 +0100 Date: Tue, 22 Jan 2019 08:33:36 +0100 From: Greg KH Subject: [MODERATED] Re: [PATCH v5 16/27] MDSv5 10 Message-ID: <20190122073336.GB7082@kroah.com> References: MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit To: speck@linutronix.de List-ID: On Fri, Jan 18, 2019 at 04:50:31PM -0800, speck for Andi Kleen wrote: > From: Andi Kleen > Subject: mds: Mark interrupts clear cpu, unless opted-out > > Interrupts might touch user data from other processes > in any context. > > By default we clear the CPU on the next kernel exit. > > Add a new IRQ_F_NO_USER interrupt flag. When the flag > is not set on interrupt execution we clear the cpu state on > next kernel exit. > > This allows interrupts to opt-out from the extra clearing > overhead, but is safe by default. > > Over time as more interrupt code is audited it can set the opt-out. > > Signed-off-by: Andi Kleen > --- > include/linux/interrupt.h | 2 ++ > kernel/irq/handle.c | 8 ++++++++ > kernel/irq/manage.c | 1 + > 3 files changed, 11 insertions(+) > > diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h > index c672f34235e7..291b7fee3afe 100644 > --- a/include/linux/interrupt.h > +++ b/include/linux/interrupt.h > @@ -61,6 +61,7 @@ > * interrupt handler after suspending interrupts. For system > * wakeup devices users need to implement wakeup detection in > * their interrupt handlers. > + * IRQF_NO_USER - Interrupt does not touch user data > */ > #define IRQF_SHARED 0x00000080 > #define IRQF_PROBE_SHARED 0x00000100 > @@ -74,6 +75,7 @@ > #define IRQF_NO_THREAD 0x00010000 > #define IRQF_EARLY_RESUME 0x00020000 > #define IRQF_COND_SUSPEND 0x00040000 > +#define IRQF_NO_USER 0x00080000 I know you want to be "safe", but I think Linus's comment about having this be "opt in" is better from an auditing point of view in that you should mark an IRQ as touching user data as you should know that you are doing that. > --- a/kernel/irq/handle.c > +++ b/kernel/irq/handle.c > @@ -13,6 +13,7 @@ > #include > #include > #include > +#include > > #include > > @@ -149,6 +150,13 @@ irqreturn_t __handle_irq_event_percpu(struct irq_desc *desc, unsigned int *flags > res = action->handler(irq, action->dev_id); > trace_irq_handler_exit(irq, action, res); > > + /* > + * We aren't sure if the interrupt handler did or did not > + * touch user data. Schedule a cpu clear just in case. > + */ > + if (!(action->flags & IRQF_NO_USER)) > + lazy_clear_cpu(); We should be sure. Why can't we be sure? We know what the irq did, right, we wrote it :) thanks, greg k-h