From 6eb46d1f1f678794fc2765024541561d84775d6e Mon Sep 17 00:00:00 2001 From: Dylan Reimerink Date: Wed, 21 Feb 2024 16:23:52 +0100 Subject: [PATCH 1/2] btf: Take instruction size into account when handling poison relocation A CO-RE relocation might be poisoned, in such cases we write a "bogus" instruction to the relocation target. This can still result in working code if that instruction turns out to be in a dead code branch due to other CO-RE logic. Currently this bogus instructions is always a function call to a non-existing function. This is a problem when the original instruction is a dword load immediate, which takes 2 instructions worth of space. When we replace this with a function call, we shrink the instruction stream which throws off offsets and instruction metadata. So this commit makes makes the CO-RE logic check if we are dealing with a dword load immediate, and if so, we replace it with a dword imm load of 0xbad2310 to R10, which is illigal and will trip the verifier if evaluated. Signed-off-by: Dylan Reimerink --- btf/core.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/btf/core.go b/btf/core.go index 724fae4b8..e26383ba3 100644 --- a/btf/core.go +++ b/btf/core.go @@ -43,7 +43,15 @@ func (f *COREFixup) Apply(ins *asm.Instruction) error { if f.poison { const badRelo = 0xbad2310 - *ins = asm.BuiltinFunc(badRelo).Call() + // Relocation is poisoned, replace the instruction with an invalid one. + + if ins.OpCode.IsDWordLoad() { + // Replace a dword load with a invalid dword load to preserve instruction size. + *ins = asm.LoadImm(asm.R10, badRelo, asm.DWord) + } else { + // Replace all single size instruction with a invalid call instruction. + *ins = asm.BuiltinFunc(badRelo).Call() + } return nil } From fe3cc27b558a9f15be8fcc99a2c4157db76b9d09 Mon Sep 17 00:00:00 2001 From: Dylan Reimerink Date: Wed, 21 Feb 2024 11:35:24 +0100 Subject: [PATCH 2/2] btf: Make test for ld64imm CO-RE relocations Added a test which makes sure we do not mess up offsets when replacing a ld64imm with poisoned instructions. Signed-off-by: Dylan Reimerink --- Makefile | 1 + btf/core_reloc_test.go | 27 +++++++++++++++++++++++++++ btf/testdata/relocs_enum-eb.elf | Bin 0 -> 1688 bytes btf/testdata/relocs_enum-el.elf | Bin 0 -> 1688 bytes btf/testdata/relocs_enum.c | 16 ++++++++++++++++ 5 files changed, 44 insertions(+) create mode 100644 btf/testdata/relocs_enum-eb.elf create mode 100644 btf/testdata/relocs_enum-el.elf create mode 100644 btf/testdata/relocs_enum.c diff --git a/Makefile b/Makefile index eb532b2fd..3727a6e11 100644 --- a/Makefile +++ b/Makefile @@ -47,6 +47,7 @@ TARGETS := \ btf/testdata/relocs \ btf/testdata/relocs_read \ btf/testdata/relocs_read_tgt \ + btf/testdata/relocs_enum \ cmd/bpf2go/testdata/minimal .PHONY: all clean container-all container-shell generate diff --git a/btf/core_reloc_test.go b/btf/core_reloc_test.go index d6c9dd00f..798676007 100644 --- a/btf/core_reloc_test.go +++ b/btf/core_reloc_test.go @@ -110,3 +110,30 @@ func TestCORERelocationRead(t *testing.T) { } }) } + +func TestLD64IMMReloc(t *testing.T) { + testutils.SkipOnOldKernel(t, "5.4", "vmlinux BTF in sysfs") + + testutils.Files(t, testutils.Glob(t, "testdata/relocs_enum-*.elf"), func(t *testing.T, file string) { + fh, err := os.Open(file) + if err != nil { + t.Fatal(err) + } + defer fh.Close() + + spec, err := ebpf.LoadCollectionSpecFromReader(fh) + if err != nil { + t.Fatal(err) + } + + if spec.ByteOrder != internal.NativeEndian { + return + } + + coll, err := ebpf.NewCollection(spec) + if err != nil { + t.Fatal(err) + } + defer coll.Close() + }) +} diff --git a/btf/testdata/relocs_enum-eb.elf b/btf/testdata/relocs_enum-eb.elf new file mode 100644 index 0000000000000000000000000000000000000000..01467e9a9da18321bae386ce1a562babb43e4e6d GIT binary patch literal 1688 zcmb_cQEL-H5S~j?HCCaBLeQ6^4|0KWiKWzss)Q8dgTa=j1qESqxopbeB^Pe@LW>XP z2l!ABpA^J*-~A2#7Jq;*LY&#`rrR?pfb=+?7Zl(-Myn{hj0&Syf+@ydU8;cT*!VP zgIdbtBovH~vY{UWV}ge=N=DxLl3v!UF1GC2?Ri!AHI$=)>*3B!MQAHF3F3@1ei8{O zJYwAZIX2vj2hp8Z)rMup0x8p|mr2f;=Vc>2b01@2G7{3AByk|4n5%ZMl>5+KQvToS z?m#)^GD}BpeFx0_yLe9|H?`6nbWt}@ub|#QrTZkI`J$;Kj_#Z!MSV9%s0qj+fL`enb{=zc72*RQ$Lqhm}=rnE?}>}jIpwgH!kKC&orhvp`KSX sHvO+5_L5m!z5l5Wo|{FPe1A56g!$$_b-q-2#kwZCXlaePXH&oO3-togvj6}9 literal 0 HcmV?d00001 diff --git a/btf/testdata/relocs_enum-el.elf b/btf/testdata/relocs_enum-el.elf new file mode 100644 index 0000000000000000000000000000000000000000..d08985416898715cd5a2cef9eac076e5941d0197 GIT binary patch literal 1688 zcmbtVQEL-H5S}Djja4W`LF&uV2f0AGL@D*5Do2X(!C*_%fP%2O+-=I?Uy+Y7=2xAF;l5!RWd%NLq2qN)cb6H=~{L?haMoeUVaV}UG zfIrFvmkjmM@03MZAOGOs>G5Irh;_Qn&dUxvIP5+@fqPKr{n4P_mxH?GLI$4n>M4&B zUoakLL&pck1ovf_#P;@zUbbr%wru6wvuoBHsD=Z}#>`B4C>0xfQN|fR3xyOmDOT|w zJJ#|=bmujdu*#SxWg7M~$r-clEXF$r=<}0UNNb!#o(v)s)LD zjjhH$0KI>U-VxeO4Vr--Vh3>*v5BDfNk-GPfgsI{WQzOu2s%JM;@=Q!4FFs7Ell9- z0-&V)7s$&G7rE4&ppSBn5Z4eimxiz7{%=-M-7QBwQ?#P$<~?_iZ06!!?W8-CQC+&T ziy$-nXh!F}SK!n;nTdZlqjUbLz}LZ%{7iy_^XW8CS`LvFBMD9vjfcz&f>ea3;0T#Y zuaE2bP_1|r+RbKz-PZ;FbJ@f5voFbVBJ~kK>3CV5&xeue*%8)^pjWrR4W;LmH=4yi zop