linux-kernel - Re: [PATCH v6.6 RESEND 2/2] x86/irq: Plug vector setup race

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] [day] [month] [year] [list]

Message-ID: <2025082244-halves-enjoyment-9417@gregkh>
Date: Fri, 22 Aug 2025 11:50:40 +0200
From: Greg KH <gregkh@...uxfoundation.org>
To: Jinjie Ruan <ruanjinjie@...wei.com>
Cc: tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
	dave.hansen@...ux.intel.com, hpa@...or.com, prarit@...hat.com,
	x86@...nel.org, stable@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6.6 RESEND 2/2] x86/irq: Plug vector setup race

On Fri, Aug 22, 2025 at 11:48:56AM +0200, Greg KH wrote:
> On Fri, Aug 22, 2025 at 05:25:56PM +0800, Jinjie Ruan wrote:
> > 
> > 
> > On 2025/8/22 16:57, Greg KH wrote:
> > > On Fri, Aug 22, 2025 at 03:38:25AM +0000, Jinjie Ruan wrote:
> > >> From: Thomas Gleixner <tglx@...utronix.de>
> > >>
> > >> commit ce0b5eedcb753697d43f61dd2e27d68eb5d3150f upstream.
> > >>
> > 
> > [...]
> > 
> > >>  
> > >>  /*
> > >> @@ -273,7 +308,9 @@ DEFINE_IDTENTRY_IRQ(common_interrupt)
> > >>  	/* entry code tells RCU that we're not quiescent.  Check it. */
> > >>  	RCU_LOCKDEP_WARN(!rcu_is_watching(), "IRQ failed to wake up RCU");
> > >>  
> > >> -	call_irq_handler(vector, regs);
> > >> +	if (unlikely(!call_irq_handler(vector, regs)))
> > >> +		apic_eoi();
> > >> +
> > > 
> > > This chunk does not look correct.  The original commit did not have
> > > this, so why add it here?  Where did it come from?
> > > 
> > > The original patch said:
> > > 	-       if (unlikely(call_irq_handler(vector, regs)))
> > > 	+       if (unlikely(!call_irq_handler(vector, regs)))
> > > 
> > > And was not an if statement.
> > > 
> > > So did you forget to backport something else here?  Why is this not
> > > identical to what the original was?
> > 
> > The if statement is introduced in commit 1b03d82ba15e ("x86/irq: Install
> > posted MSI notification handler") which is a patch in patch set
> > https://lore.kernel.org/all/20240423174114.526704-1-jacob.jun.pan@linux.intel.com/,
> > but it seems to be a performance optimization patch set, and this patch
> > includes additional modifications. The context conflict is merely a
> > simple refactoring, but the cost of the entire round of this patch set
> > is too high.
> 
> Why is it "too high"?  We almost NEVER want to deviate from what is in
> mainline, as every time wo do that it adds the potential for bugs AND it
> increases our maintance burden (i.e. later patches will not apply.)
> 
> For a kernel that has to live as long as this one does, we need to try
> to stick to what is in mainline as close as possible.  Otherwise it
> becomes unworkable.

Also, I will push back, why not just use 6.12.y to resolve this issue
instead?  Why are you stuck on 6.6.y for now, and what are you going to
do with those systems once 6.6.y goes end-of-life?  Why postpone the
inevitable?

thanks,

greg k-h