From 7e1d7f96d54ba4c14fe4428adab262947c80e8a0 Mon Sep 17 00:00:00 2001 From: Timo Beckers Date: Fri, 17 Mar 2023 10:48:16 +0100 Subject: [PATCH 1/2] link/cgroup: clone prog handle and skip closing cgroup for newProgAttachCgroup Prog only needs to be cloned in newProgAttachCgroup. Remove the Close() footgun, making future modifications less tricky. Execute PROG_ATTACH using the cloned program for good measure. Signed-off-by: Timo Beckers --- link/cgroup.go | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/link/cgroup.go b/link/cgroup.go index a685fe09d..a1f9667a7 100644 --- a/link/cgroup.go +++ b/link/cgroup.go @@ -27,36 +27,33 @@ type CgroupOptions struct { } // AttachCgroup links a BPF program to a cgroup. -func AttachCgroup(opts CgroupOptions) (Link, error) { +func AttachCgroup(opts CgroupOptions) (cg Link, err error) { cgroup, err := os.Open(opts.Path) if err != nil { return nil, fmt.Errorf("can't open cgroup: %s", err) } - - clone, err := opts.Program.Clone() - if err != nil { + defer func() { + if _, ok := cg.(*progAttachCgroup); ok { + // Skip closing the cgroup handle if we return a valid progAttachCgroup, + // where the handle is retained to implement Update(). + return + } cgroup.Close() - return nil, err - } + }() - var cg Link - cg, err = newLinkCgroup(cgroup, opts.Attach, clone) + cg, err = newLinkCgroup(cgroup, opts.Attach, opts.Program) if err == nil { - cgroup.Close() - clone.Close() return cg, nil } // cgroup and clone are retained by progAttachCgroup. if errors.Is(err, ErrNotSupported) { - cg, err = newProgAttachCgroup(cgroup, opts.Attach, clone, flagAllowMulti) + cg, err = newProgAttachCgroup(cgroup, opts.Attach, opts.Program, flagAllowMulti) } if errors.Is(err, ErrNotSupported) { - cg, err = newProgAttachCgroup(cgroup, opts.Attach, clone, flagAllowOverride) + cg, err = newProgAttachCgroup(cgroup, opts.Attach, opts.Program, flagAllowOverride) } if err != nil { - cgroup.Close() - clone.Close() return nil, err } @@ -83,17 +80,24 @@ func newProgAttachCgroup(cgroup *os.File, attach ebpf.AttachType, prog *ebpf.Pro } } - err := RawAttachProgram(RawAttachProgramOptions{ + // Use a program handle that cannot be closed by the caller. + clone, err := prog.Clone() + if err != nil { + return nil, err + } + + err = RawAttachProgram(RawAttachProgramOptions{ Target: int(cgroup.Fd()), - Program: prog, + Program: clone, Flags: uint32(flags), Attach: attach, }) if err != nil { + clone.Close() return nil, fmt.Errorf("cgroup: %w", err) } - return &progAttachCgroup{cgroup, prog, attach, flags}, nil + return &progAttachCgroup{cgroup, clone, attach, flags}, nil } func (cg *progAttachCgroup) Close() error { From fa35bf066c89e3a372d07c849a9487a6de13de0d Mon Sep 17 00:00:00 2001 From: Timo Beckers Date: Tue, 21 Mar 2023 17:23:01 +0100 Subject: [PATCH 2/2] link/cgroup: extend AttachCgroup docs and document attachment flags Also addressed an inaccuracy in the QueryPrograms docstring. bpf_link progs are returned through PROG_QUERY. Clarify that BPF_F_REPLACE only affects programs attached with the Multi flag. Signed-off-by: Timo Beckers --- link/cgroup.go | 15 +++++++++++++-- link/query.go | 6 +++--- link/syscalls.go | 2 +- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/link/cgroup.go b/link/cgroup.go index a1f9667a7..58e85fe9d 100644 --- a/link/cgroup.go +++ b/link/cgroup.go @@ -10,10 +10,15 @@ import ( type cgroupAttachFlags uint32 -// cgroup attach flags const ( + // Allow programs attached to sub-cgroups to override the verdict of this + // program. flagAllowOverride cgroupAttachFlags = 1 << iota + // Allow attaching multiple programs to the cgroup. Only works if the cgroup + // has zero or more programs attached using the Multi flag. Implies override. flagAllowMulti + // Set automatically by progAttachCgroup.Update(). Used for updating a + // specific given program attached in multi-mode. flagReplace ) @@ -27,6 +32,13 @@ type CgroupOptions struct { } // AttachCgroup links a BPF program to a cgroup. +// +// If the running kernel doesn't support bpf_link, attempts to emulate its +// semantics using the legacy PROG_ATTACH mechanism. If bpf_link is not +// available, the returned [Link] will not support pinning to bpffs. +// +// If you need more control over attachment flags or the attachment mechanism +// used, look at [RawAttachProgram] and [AttachRawLink] instead. func AttachCgroup(opts CgroupOptions) (cg Link, err error) { cgroup, err := os.Open(opts.Path) if err != nil { @@ -46,7 +58,6 @@ func AttachCgroup(opts CgroupOptions) (cg Link, err error) { return cg, nil } - // cgroup and clone are retained by progAttachCgroup. if errors.Is(err, ErrNotSupported) { cg, err = newProgAttachCgroup(cgroup, opts.Attach, opts.Program, flagAllowMulti) } diff --git a/link/query.go b/link/query.go index 8c882414d..c05656512 100644 --- a/link/query.go +++ b/link/query.go @@ -21,9 +21,9 @@ type QueryOptions struct { // QueryPrograms retrieves ProgramIDs associated with the AttachType. // -// It only returns IDs of programs that were attached using PROG_ATTACH and not bpf_link. -// Returns (nil, nil) if there are no programs attached to the queried kernel resource. -// Calling QueryPrograms on a kernel missing PROG_QUERY will result in ErrNotSupported. +// Returns (nil, nil) if there are no programs attached to the queried kernel +// resource. Calling QueryPrograms on a kernel missing PROG_QUERY will result in +// ErrNotSupported. func QueryPrograms(opts QueryOptions) ([]ebpf.ProgramID, error) { if haveProgQuery() != nil { return nil, fmt.Errorf("can't query program IDs: %w", ErrNotSupported) diff --git a/link/syscalls.go b/link/syscalls.go index 38f7ae9b7..c9c998c20 100644 --- a/link/syscalls.go +++ b/link/syscalls.go @@ -46,7 +46,7 @@ var haveProgAttach = internal.NewFeatureTest("BPF_PROG_ATTACH", "4.10", func() e return nil }) -var haveProgAttachReplace = internal.NewFeatureTest("BPF_PROG_ATTACH atomic replacement", "5.5", func() error { +var haveProgAttachReplace = internal.NewFeatureTest("BPF_PROG_ATTACH atomic replacement of MULTI progs", "5.5", func() error { if err := haveProgAttach(); err != nil { return err }