From: Ingo Molnar <mingo@kernel.org>
To: Borislav Petkov <bp@alien8.de>, Mike Galbraith <efault@gmx.de>
Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>,
Thomas Gleixner <tglx@linutronix.de>,
LKML <linux-kernel@vger.kernel.org>,
"H. Peter Anvin" <hpa@zytor.com>
Subject: Re: [PATCH] x86/microcode: Do not access the initrd after it has been freed
Date: Tue, 31 Jan 2017 08:43:55 +0100 [thread overview]
Message-ID: <20170131074355.GA17636@gmail.com> (raw)
In-Reply-To: <20170130093527.syjz7yldgndml7qb@pd.tnic>
(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;
next prev parent reply other threads:[~2017-01-31 7:45 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-01-25 16:58 x86/microcode: use-after-free after cpu offline/online Andrey Ryabinin
2017-01-25 17:23 ` Borislav Petkov
2017-01-25 19:14 ` Andrey Ryabinin
2017-01-25 19:23 ` Borislav Petkov
2017-01-26 16:58 ` [PATCH] x86/microcode: Do not access the initrd after it has been freed Borislav Petkov
2017-01-27 8:14 ` Andrey Ryabinin
2017-01-27 9:09 ` Borislav Petkov
2017-01-30 8:46 ` Ingo Molnar
2017-01-30 9:35 ` Borislav Petkov
2017-01-31 7:43 ` Ingo Molnar [this message]
2017-01-31 10:01 ` Borislav Petkov
2017-01-31 11:31 ` Mike Galbraith
2017-01-31 12:31 ` Borislav Petkov
2017-01-31 17:49 ` Borislav Petkov
2017-01-31 18:05 ` Mike Galbraith
2017-01-31 18:03 ` Thomas Gleixner
2017-01-31 19:25 ` [tip:irq/urgent] x86/irq: Make irq activate operations symmetric tip-bot for Thomas Gleixner
2017-01-31 16:39 ` [PATCH] x86/microcode: Do not access the initrd after it has been freed Ingo Molnar
2017-01-30 8:49 ` [tip:x86/microcode] " tip-bot for Borislav Petkov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170131074355.GA17636@gmail.com \
--to=mingo@kernel.org \
--cc=aryabinin@virtuozzo.com \
--cc=bp@alien8.de \
--cc=efault@gmx.de \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).