Messages in this thread | | | Date | Tue, 23 Aug 2022 11:13:13 +0000 | From | Ashok Raj <> | Subject | Re: [PATCH v3 1/5] x86/microcode/intel: Check against CPU signature before saving microcode |
| |
On Fri, Aug 19, 2022 at 12:24:41PM +0200, Borislav Petkov wrote: > On Wed, Aug 17, 2022 at 05:11:23AM +0000, Ashok Raj wrote: > > When save_microcode_patch() is looking to replace an existing microcode in > > the cache, current code is *only* checks the CPU sig/pf in the main > > Write those "sig/pf" things out once so that it is clear what that is.
Thanks, will do. > > > header. Microcode can carry additional sig/pf combinations in the extended > > signature table, which is completely missed today. > > > > For e.g. Current patch is a multi-stepping patch and new incoming patch is > > a specific patch just for this CPUs stepping. > > > > patch1: > > fms3 <--- header FMS > > ... > > ext_sig: > > fms1 > > fms2 > > > > patch2: new > > fms2 <--- header FMS > > > > Current code takes only fms3 and checks with patch2 fms2. > > So, find_matching_signature() does all the signatures matching and > scanning already. If anything, that function should tell its callers > whether the patch it is looking at - the fms2 one - should replace the > current one or not. > > I.e., all the logic to say how strong a patch match is, should be > concentrated there. And then the caller will do the according action.
I updated the commit log accordingly. Basically find_matching_signature() is only intended to find a CPU's sig/pf against a microcode image and not intended to compare between two different images. > > > saved_patch.header.fms3 != new_patch.header.fms2, so save_microcode_patch > > saves it to the end of list instead of replacing patch1 with patch2. > > > > There is no functional user observable issue since find_patch() skips > > patch versions that are <= current_patch and will land on patch2 properly. > > > > Nevertheless this will just end up storing every patch that isn't required. > > Kernel just needs to store the latest patch. Otherwise its a memory leak > > that sits in kernel and never used. > > Oh well. > > > Cc: stable@vger.kernel.org > > Why?
We have some code to support model specific microcode rollback support. This code is just internal. That codebase triggered the bug.
I'll drop the Cc next time.
Cheers, Ashok
| |