From 2d6e96dfe1ddd1a62970b4e47bdd08e5d135b078 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guillermo=20Juli=C3=A1n?= Date: Tue, 9 Jan 2024 13:31:48 +0100 Subject: [PATCH] Support multiple instruction patchers (#168) * Allow multiple instruction patchers * Add tests for instruction patchers --- examples/instruction_patching/main.go | 2 +- manager.go | 11 +++-- manager_test.go | 60 ++++++++++++++++++++++++++ testdata/Makefile | 2 +- testdata/patching.c | 14 ++++++ testdata/patching.elf | Bin 0 -> 3264 bytes 6 files changed, 83 insertions(+), 6 deletions(-) create mode 100644 testdata/patching.c create mode 100644 testdata/patching.elf diff --git a/examples/instruction_patching/main.go b/examples/instruction_patching/main.go index cf9254e..1413cea 100644 --- a/examples/instruction_patching/main.go +++ b/examples/instruction_patching/main.go @@ -25,7 +25,7 @@ var m1 = &manager.Manager{ }, }, }, - InstructionPatcher: patchBPFTelemetry, + InstructionPatchers: []manager.InstructionPatcherFunc{patchBPFTelemetry}, } const BPFTelemetryPatchCall = -1 diff --git a/manager.go b/manager.go index 217dfbc..dcd1172 100644 --- a/manager.go +++ b/manager.go @@ -124,6 +124,9 @@ type Options struct { SkipRingbufferReaderStartup map[string]bool } +// InstructionPatcherFunc - A function that patches the instructions of a program +type InstructionPatcherFunc func(m *Manager) error + // Manager - Helper structure that manages multiple eBPF programs and maps type Manager struct { collectionSpec *ebpf.CollectionSpec @@ -150,9 +153,9 @@ type Manager struct { // and dump the current state (human-readable) DumpHandler func(w io.Writer, manager *Manager, mapName string, currentMap *ebpf.Map) - // InstructionPatcher - Callback function called before loading probes, to + // InstructionPatchers - Callback functions called before loading probes, to // provide user the ability to perform last minute instruction patching. - InstructionPatcher func(m *Manager) error + InstructionPatchers []InstructionPatcherFunc } // DumpMaps - Write in the w writer argument human-readable info about eBPF maps @@ -558,8 +561,8 @@ func (m *Manager) InitWithOptions(elf io.ReaderAt, options Options) error { } // Patch instructions - if m.InstructionPatcher != nil { - if err := m.InstructionPatcher(m); err != nil { + for _, patcher := range m.InstructionPatchers { + if err := patcher(m); err != nil { resetManager(m) return err } diff --git a/manager_test.go b/manager_test.go index 7c4c6d7..f47d46b 100644 --- a/manager_test.go +++ b/manager_test.go @@ -201,3 +201,63 @@ func TestDumpMaps(t *testing.T) { t.Errorf("expected %s, got %s", dumpContents, output.String()) } } + +func TestInstructionPatching(t *testing.T) { + err := rlimit.RemoveMemlock() + if err != nil { + t.Fatal(err) + } + + // We want to test multiple patchers, so we'll use a generic one + // and call it twice with different constants. + // The patching.c program contains two invalid calls, with constants + // -1 and -2. We just replace them with a movimm instruction. + genericPatcher := func(m *Manager, constant int64) error { + specs, err := m.GetProgramSpecs() + if err != nil { + return err + } + for _, spec := range specs { + if spec == nil { + continue + } + iter := spec.Instructions.Iterate() + for iter.Next() { + ins := iter.Ins + + if !ins.IsBuiltinCall() { + continue + } + + if ins.Constant == constant { + *ins = asm.Mov.Imm(asm.R1, int32(0xff)).WithMetadata(ins.Metadata) + } + } + } + return nil + } + + m := &Manager{ + Probes: []*Probe{ + {ProbeIdentificationPair: ProbeIdentificationPair{EBPFFuncName: "patching_test"}}, + }, + InstructionPatchers: []InstructionPatcherFunc{ + func(m *Manager) error { return genericPatcher(m, -1) }, + func(m *Manager) error { return genericPatcher(m, -2) }, + }, + } + + f, err := os.Open("testdata/patching.elf") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = f.Close() }) + + // If any of the patchers fail, they will leave an invalid call instruction + // in the program, which will cause the verifier to fail. This allows us + // to not do any extra validation. + err = m.InitWithOptions(f, Options{}) + if err != nil { + t.Fatal(err) + } +} diff --git a/testdata/Makefile b/testdata/Makefile index d8c8de3..7efa437 100644 --- a/testdata/Makefile +++ b/testdata/Makefile @@ -1,7 +1,7 @@ LLVM_PREFIX ?= /usr/bin CLANG ?= $(LLVM_PREFIX)/clang -all: rewrite.elf exclude.elf +all: rewrite.elf exclude.elf patching.elf clean: -$(RM) *.elf diff --git a/testdata/patching.c b/testdata/patching.c new file mode 100644 index 0000000..95574ab --- /dev/null +++ b/testdata/patching.c @@ -0,0 +1,14 @@ +#include "common.h" + +char _license[] __section("license") = "MIT"; + +static void *(*bpf_patch_1)(unsigned long, ...) = (void *)-1; +static void *(*bpf_patch_2)(unsigned long, ...) = (void *)-2; + +__section("socket") +int patching_test() { + int ret = 0; + bpf_patch_1(ret); + bpf_patch_2(ret); + return 1; +} diff --git a/testdata/patching.elf b/testdata/patching.elf new file mode 100644 index 0000000000000000000000000000000000000000..f3b46cf8d800c2c5d38f6bd93cd7a7643e0c7b36 GIT binary patch literal 3264 zcmbtWO=w(I6h3c$GBcTEn!jpWMMhC<(Z0!Flcte&T4^CzA*9_jo9UaGm&vn}H*w}A zwxHIHpl(GfidZOuZWLVT#)T^pS1!esD-m1?f`VfFzWeT-yxgR2JaF&1-}$-czH`pK zGq*2ZzLattG33Y}vWtuqJioiHs%2HiVfm#tejTzMP!j(`3^`5SQ9dHiFI^R9)On&b zo=ulV9(<@Y>WmGDq-G^uS{grh^h|NFm^vbocATYgk<6pAG(KMx$vzH)v&ABo16&r1 zXR*x1Zgj#455ZQXa2TFX9u*nPDqO^FGIR*ju#3oJ1JxsUE*wpl=){O3rCE7+*a%?v zIYtHpTdJ=cl1L$u+nSS2Vx-=~bPmHw7t&yg($9fqRyaxa9@v00@G)3Em-`B=iQ#1P z0a#%R)8ICyq6}rf1<&PkKY}Tnfnlr^F_g3VGv~smm`m~}*MnS3_IFMhX|%ntRo?cy zy`U48r>5OWcdC4RJ&HEZR4T0?a@#@ZbvtgS+p0vJPP?)01!3g{KfDoyy_3C0H`t7N zE)2_lV_|J!=G4^u^h~{3KYeQM^!(hyLVa>}W^(S-+}h0Sv^VF?&oxiTrWZBVgRtc` zgdr+^eRJ*Ph8KD*zgvm?Uexp=PipO;;fFn65V9+^+WD20^Ve!umR2v;u3lTdSgQ%# zYHF|Al(akejIFR2v_ijG4#Fs*o$g1had)A%>qqk8ua2ZPb?-y(?$f%Ru{Ejynm+qwJdN}46W|=rU@U7H(t(S< zHzy-nW?KFja1|r=f z^H*sYHBwbVmYse^ks(C|)>+Z9Y-(_#Qc0!ani}zb&I)Gax1IeOyg_jT>{F$M_1;hE z?C8q8pA$*$-=K%_)U#bUt#<^mj0fFKeO<= z8k6yS=*T{CziJ=F`!8Ggx`lZiNcxkQKZ*SDRW)n|)Yxgx(d&XvX+Wn;4TG!j~ z`*CnRTlhyNclf7i`3klbcPKY!{cDxqMWlEfQ8ndxJrtD1$y&3!Op z-h==0H}}NY^EpeB2S9)O8#c9(9gnt2BDQ7JN80GF=7!m}m!Pm^w4KnxFN~w+w(SMT z3ziMx*ZPSX(GGkEm}B$(BQyf>KGVd`(|6kMZyF`Y#5epC=*&7{Cg1R~ffg;L1?3~y z9CZHgbwikQ^8B>3&yT-v2gX-F(a@PXVyVcEe;Cq(;{T-MS7SHzj^aP&zWA>|X75Rt zo4Rq16C}u---vEuaZvoSzCUGc$nWQ`i19ah*?{bz_;+>uXSKg!wrv2#LGi0PzKO&5 xysLKn70A+;cRFCNVf1NP`|CF7{|?#lKY)j^={sZbi$4DkT1Y=`Y2urA_+Pl;DwO~L literal 0 HcmV?d00001