Fix BPF-related fd leaks

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
 }

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
 		}