lkml.org 
[lkml]   [2022]   [Mar]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: x86, possible bug in __memmove() alternatives patching
Date
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


\
 
 \ /
  Last update: 2022-03-30 15:57    [W:0.122 / U:0.824 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site