lkml.org 
[lkml]   [2017]   [Jan]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] x86/microcode: Do not access the initrd after it has been freed

(Cc:-ed Mike as this could explain his early boot crash/hang?
Mike: please try -tip f18a8a0143b1 that I just pushed out. )

* Borislav Petkov <bp@alien8.de> wrote:

> On Mon, Jan 30, 2017 at 09:46:32AM +0100, Ingo Molnar wrote:
> > Ok, I have applied this to tip:x86/urgent.
> >
> > Note that there are new conflicts with your pending work in tip:x86/microcode, and
> > I fixed them up in:
> >
> > 7c5b4112040e Merge branch 'x86/urgent' into x86/microcode, to resolve conflicts
> >
> > Could you please double-check my conflict resolution?
>
> Almost, this part is wrong:
>
> --------------------- arch/x86/kernel/cpu/microcode/amd.c ---------------------
> index 7889ae492af0,079e81733a58..73082365ed1c
> @@@ -268,20 -316,43 +268,20 @@@ void __load_ucode_amd(unsigned int cpui
> use_pa = false;
> }
>
> - if (!get_builtin_microcode(&cp, x86_family(cpuid_1_eax)))
> - if (!get_builtin_microcode(&cp, family))
> ++ if (!get_builtin_microcode(&cp, x86_family(cpuid_1_eax)) && !initrd_gone)
> cp = find_microcode_in_initrd(path, use_pa);
>
> --
>
> Btw, I did experiment with the merging because I knew it'll cause
> trouble due to the urgent fix and here's what I did:
>
> You're merging tip/x86/urgent into tip/x86/microcode so I checked out
> the microcode branch and did:
>
> $ git checkout -b tip-microcode tip/x86/microcode
> $ git merge -s recursive -X ours tip/x86/urgent
>
> This way I'm favouring our changes in the conflicting files. It merges
> cleanly and the resulting diff is below.

Nice - I've updated the branch with your resolution. Could you please
double-check the double checked resolution?

> The logic behind it is is that tip/x86/microcode does away with a bunch
> of code and the urgent change touches some of that code but that's only
> for 4.10.
>
> It goes away in 4.11 and that's why we should prefer "ours" as the merge
> option.
>
> [ Btw, I'll send a patch for 4.11 later to make initrd_gone static as
> it is going to be used only in microcode/core.c after the cleanup. ]
>
> However, I still haven't figured out how to say "prefer ours but only
> for specific files or subtree" because the diff has that hunk in
> arch/x86/kernel/fpu/core.c too which should definitely not be "ours" as
> it is a fix and there the urgent version should be the one going in.
>
> Hmmm.

So the diff between your resolution and mine is attached below - now fpu/core.c
changes, so I'm not sure why fpu/core.c is in your diff?

Thanks,

Ingo

diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index 73082365ed1c..7889ae492af0 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -268,7 +268,7 @@ void __load_ucode_amd(unsigned int cpuid_1_eax, struct cpio_data *ret)
use_pa = false;
}

- if (!get_builtin_microcode(&cp, x86_family(cpuid_1_eax)) && !initrd_gone)
+ if (!get_builtin_microcode(&cp, x86_family(cpuid_1_eax)))
cp = find_microcode_in_initrd(path, use_pa);

/* Needed in load_microcode_amd() */
diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
index e51eeaed8016..b4a4cd39b358 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -230,7 +230,7 @@ static int __init save_microcode_in_initrd(void)
break;
case X86_VENDOR_AMD:
if (c->x86 >= 0x10)
- ret = save_microcode_in_initrd_amd(cpuid_eax(1));
+ return save_microcode_in_initrd_amd(cpuid_eax(1));
break;
default:
break;
\
 
 \ /
  Last update: 2017-01-31 08:45    [W:0.087 / U:1.104 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site