Messages in this thread | | | From | Matthias Welwarsky <> | Subject | Re: x86, possible bug in __memmove() alternatives patching | Date | Wed, 30 Mar 2022 15:56:52 +0200 |
| |
On Mittwoch, 30. März 2022 00:33:17 CEST Dave Hansen wrote: > On 3/26/22 04:39, Matthias Welwarsky wrote: > >> But, we do try to make the kernel work even the face of funky > >> hypervisors that do things that never occur on real hardware. If a nice > >> patch to fix this up showed up, I'd definitely take a look. > > > > The question is whether a sequence like this could be relevant: > > > > 0) CPU announces feature FSRM through cpuid > > 1) BIOS/firmware disables fast string ops through IA32_MISC_ENABLE before > > loading kernel (for whatever reason) > > 2) Kernel populates features from cpuid > > 3) Kernel clears ERMS based on IA32_MISC_ENABLE > > 4) "alternatives" patching destroys __memmove() > > Hi Matthias, > > What does "destroys __memmove()" mean in practice? What's the end-user > visible effect of this? Do they see a crash or just crummy performance?
Solid kernel freeze in my case. No Oops, boot just hangs right after __memmove() was patched. Not easily trackable.
I'll send a patch. I think the same rationale applies to FSRM as to ERMS, which gets manually cleared when IA32_MISC_ENABLE says that fast string ops are not available. It will be a one liner added to the dependency table in cpu-deps.c, making FSRM depend on ERMS so that it gets automatically cleared.
The way __memmove gets broken is kind of obvious if you look at the code.
Here's the relevant bits:
/* FSRM implies ERMS => no length checks, do the copy directly */ .Lmemmove_begin_forward: ALTERNATIVE "cmp $0x20, %rdx; jb 1f", "", X86_FEATURE_FSRM ALTERNATIVE "", __stringify(movq %rdx, %rcx; rep movsb; RET), X86_FEATURE_ERMS
If FSRM is there but ERMS isn't, the first ALTERNATIVE is activated but not the second one. That means the length check (< 32) and subsequent "jb 1f" is suppressed but the "movq %rdx, %rcx; rep movsb; RET" is also not there.
So, if the amount to be moved is < 32, it executes a code path that relies on having at least 32 byte to copy and that results in doing all kinds of stuff but not what you'd expect. I haven't analyzed what happens in depth, but it might move more data than requested, or nothing at all.
-- Mit freundlichen Grüßen/Best regards,
Matthias Welwarsky Project Engineer
SYSGO GmbH Office Mainz Am Pfaffenstein 8 / D-55270 Klein-Winternheim / Germany Phone: +49-6136-9948-0 / Fax: +49-6136-9948-10 E-mail: matthias.welwarsky@sysgo.com _________________________________________________________________________________ Web: https://www.sysgo.com Blog: https://www.sysgo.com/blog Events: https://www.sysgo.com/events Newsletter: https://www.sysgo.com/newsletter _________________________________________________________________________________ Handelsregister/Commercial Registry: HRB Mainz 90 HRB 48884 Geschäftsführung/Managing Directors: Etienne Butery (CEO), Kai Sablotny (COO) USt-Id-Nr./VAT-Id-No.: DE 149062328
The protection of your personal data is important to us. Under the following link you can see the information in accordance with article 13 GDPR: https://www.sysgo.com/privacy_policy
| |