internal/pe1.14: support arm64 and riscv64 #204
Conversation
|
There is still the problem of shimaa64.efi not having a |
| var pe64 bool | ||
| switch f.Machine { | ||
| case IMAGE_FILE_MACHINE_AMD64, IMAGE_FILE_MACHINE_ARM64, IMAGE_FILE_MACHINE_RISCV64: | ||
| pe64 = true |
There was a problem hiding this comment.
I was wondering what defines whether a machine types uses the PE32 or PE32+ optional header, and then I realised that this check is probably wrong. The type of the optional header (PE32 vs PE32+) is determined by the first uint16 value in the header, which is checked already when the header is decoded in readOptionalHeader. The code here probably shouldn't check the machine type but instead just check whether the underlying type of f.OptionalHeader is *OptionalHeader32 or *OptionalHeader64.
This is probably ok for now because this code is inherited from the upstream go repo, but it might be worth reporting a bug there.
There was a problem hiding this comment.
Oh, I hadn't noticed this was actually a copy of a go package. FWIW, very similar changes to what I did were performed in golang/go@0ca0551. Unfortunately that change is in go 1.17 and we still must support go 1.13 afaiu.
You are right in that the way PE32+ is detected is not fully right, and might fail in some corner case. I have opened an issue for this: golang/go#54250.
I have also opened golang/go#54251 to ask for RISCV64 support, that is still not in upstream.
|
@chrisccoulson thanks for the review. I also have an identical change in canonical/go-efilib#9 - that is being pulled as a dependency too by snapd, even though it looks like it is duplicated code. So we would need that change too, unless the code duplication is removed in some way. |
No description provided.