linux-kernel - Re: [PATCH] x86/apic: Fix the issues in x2apic

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: <20240812155358.890025-1-yuntao.wang@linux.dev>
Date: Mon, 12 Aug 2024 23:53:58 +0800
From: Yuntao Wang <yuntao.wang@...ux.dev>
To: tglx@...utronix.de
Cc: bp@...en8.de,
	daniel.sneddon@...ux.intel.com,
	dave.hansen@...ux.intel.com,
	hpa@...or.com,
	linux-kernel@...r.kernel.org,
	mingo@...hat.com,
	peterz@...radead.org,
	thorsten.blum@...lux.com,
	tony.luck@...el.com,
	x86@...nel.org,
	yuntao.wang@...ux.dev
Subject: Re: [PATCH] x86/apic: Fix the issues in x2apic_disable()

On Mon, 12 Aug 2024 16:48:05 +0200, Thomas Gleixner <tglx@...utronix.de> wrote:

> On Mon, Aug 12 2024 at 18:08, Yuntao Wang wrote:
> >  static __init void x2apic_disable(void)
> >  {
> > -	u32 x2apic_id, state = x2apic_state;
> > +	u32 x2apic_id;
> >  
> > -	x2apic_mode = 0;
> > -	x2apic_state = X2APIC_DISABLED;
> > -
> > -	if (state != X2APIC_ON)
> > -		return;
> > +	if (x2apic_state < X2APIC_ON)
> > +		goto out;
> 
> There is no point in overwriting the state in case it is < ON, no?

Are you saying that we should replace 'goto out' with a return statement?

However, when x2apic_disable() is called, it's possible that x2apic_state
is X2APIC_OFF. In that case, we should set x2apic_state to X2APIC_DISABLED.

So, I think overwriting the state is necessary.

Thanks,
Yuntao

> Thanks,
> 
>         tglx