rhboot / shim

UEFI shim loader
Other
848 stars 290 forks source link

GRUB crashing due to apparently stale instruction cache #498

Closed dannf closed 1 year ago

dannf commented 2 years ago

We're seeing intermittent (~1/100) Synchronous Exceptions in early GRUB code on an ARM Cortex-A72-based platform. GRUB is being loaded by shim (Secure Boot is disabled fwiw). Curiously, the instruction $IP points to never seems like something that should cause an exception. One theory was that the instruction cache contained stale content, which seemed to be confirmed by manually flushing the cache within a debugger after hitting the exception, and re-executing the same code. This worked. Our working theory is that shim should be flushing the I-cache after loading the GRUB binary. (This story sounds pretty similar, btw).

We added the following code to a test shim build, and the test is no longer failing. Multiple machines have survived over 1000 reboots:

--- pe.c~       2022-07-28 23:31:51.399506464 +0000
+++ pe.c        2022-07-28 23:51:25.398746245 +0000
@@ -963,6 +963,7 @@

        CopyMem(buffer, data, context.SizeOfHeaders);

+       __builtin___clear_cache((char *)buffer, (char *)(buffer + context.ImageSize));
        *entry_point = ImageAddress(buffer, context.ImageSize, context.EntryPoint);
        if (!*entry_point) {
                perror(L"Entry point is invalid\n");
frozencemetery commented 2 years ago

Thank you for debugging into this. Could you please open this as a PR so we can review it for inclusion?

dannf commented 2 years ago

Thank you for debugging into this. Could you please open this as a PR so we can review it for inclusion?

Thanks for the reply @frozencemetery. What I have above is only a POC, not something I want to propose for merging. I'm working on a better implementation for which I'll submit a PR when ready.

julian-klode commented 1 year ago

The commit got merged but I guess github didn't understand that it fixed the bug and only thought it referenced it, so closing manually.