diff --git a/devices/ebpf_linux.go b/devices/ebpf_linux.go index 6a41aff..cfc36f7 100644 --- a/devices/ebpf_linux.go +++ b/devices/ebpf_linux.go @@ -15,11 +15,7 @@ import ( "golang.org/x/sys/unix" ) -func nilCloser() error { - return nil -} - -func findAttachedCgroupDeviceFilters(dirFd int) ([]*ebpf.Program, error) { +func findAttachedCgroupDeviceFilters(dirFd int) (_ []*ebpf.Program, retErr error) { type bpfAttrQuery struct { TargetFd uint32 AttachType uint32 @@ -58,8 +54,17 @@ func findAttachedCgroupDeviceFilters(dirFd int) ([]*ebpf.Program, error) { } // Convert the ids to program handles. + // On error we don't return the programs slice, so close the fds stored there. progIds = progIds[:size] programs := make([]*ebpf.Program, 0, len(progIds)) + defer func() { + if retErr != nil { + for _, p := range programs { + p.Close() + } + } + }() + for _, progId := range progIds { program, err := ebpf.NewProgramFromID(ebpf.ProgramID(progId)) if err != nil { @@ -154,7 +159,7 @@ func haveBpfProgReplace() bool { // Requires the system to be running in cgroup2 unified-mode with kernel >= 4.15 . // // https://github.com/torvalds/linux/commit/ebc614f687369f9df99828572b1d85a7c2de3d92 -func loadAttachCgroupDeviceFilter(insts asm.Instructions, license string, dirFd int) (func() error, error) { +func loadAttachCgroupDeviceFilter(insts asm.Instructions, license string, dirFd int) error { // Increase `ulimit -l` limit to avoid BPF_PROG_LOAD error (#2167). // This limit is not inherited into the container. memlockLimit := &unix.Rlimit{ @@ -166,8 +171,14 @@ func loadAttachCgroupDeviceFilter(insts asm.Instructions, license string, dirFd // Get the list of existing programs. oldProgs, err := findAttachedCgroupDeviceFilters(dirFd) if err != nil { - return nilCloser, err + return err } + defer func() { + for _, p := range oldProgs { + p.Close() + } + }() + useReplaceProg := haveBpfProgReplace() && len(oldProgs) == 1 // Generate new program. @@ -178,8 +189,9 @@ func loadAttachCgroupDeviceFilter(insts asm.Instructions, license string, dirFd } prog, err := ebpf.NewProgram(spec) if err != nil { - return nilCloser, err + return err } + defer prog.Close() // If there is only one old program, we can just replace it directly. @@ -195,20 +207,7 @@ func loadAttachCgroupDeviceFilter(insts asm.Instructions, license string, dirFd } err = link.RawAttachProgram(attachProgramOptions) if err != nil { - return nilCloser, fmt.Errorf("failed to call BPF_PROG_ATTACH (BPF_CGROUP_DEVICE, BPF_F_ALLOW_MULTI): %w", err) - } - closer := func() error { - err = link.RawDetachProgram(link.RawDetachProgramOptions{ - Target: dirFd, - Program: prog, - Attach: ebpf.AttachCGroupDevice, - }) - if err != nil { - return fmt.Errorf("failed to call BPF_PROG_DETACH (BPF_CGROUP_DEVICE): %w", err) - } - // TODO: Should we attach the old filters back in this case? Otherwise - // we fail-open on a security feature, which is a bit scary. - return nil + return fmt.Errorf("failed to call BPF_PROG_ATTACH (BPF_CGROUP_DEVICE, BPF_F_ALLOW_MULTI): %w", err) } if !useReplaceProg { logLevel := logrus.DebugLevel @@ -248,9 +247,9 @@ func loadAttachCgroupDeviceFilter(insts asm.Instructions, license string, dirFd Attach: ebpf.AttachCGroupDevice, }) if err != nil { - return closer, fmt.Errorf("failed to call BPF_PROG_DETACH (BPF_CGROUP_DEVICE) on old filter program: %w", err) + return fmt.Errorf("failed to call BPF_PROG_DETACH (BPF_CGROUP_DEVICE) on old filter program: %w", err) } } } - return closer, nil + return nil } diff --git a/devices/v2.go b/devices/v2.go index d54298f..508f3dd 100644 --- a/devices/v2.go +++ b/devices/v2.go @@ -64,7 +64,7 @@ func setV2(dirPath string, r *cgroups.Resources) error { return fmt.Errorf("cannot get dir FD for %s", dirPath) } defer unix.Close(dirFD) - if _, err := loadAttachCgroupDeviceFilter(insts, license, dirFD); err != nil { + if err := loadAttachCgroupDeviceFilter(insts, license, dirFD); err != nil { if !canSkipEBPFError(r) { return err }