ebpf icon indicating copy to clipboard operation
ebpf copied to clipboard

proposal: sys: make UAPI wrappers type safe(er)

Open lmb opened this issue 2 years ago • 0 comments

The BPF uapi uses a special type __aligned_u64 to store pointers:

	struct { /* anonymous struct used by BPF_OBJ_* commands */
		__aligned_u64	pathname;
		__u32		bpf_fd;
		__u32		file_flags;
	};

The reason for this is explained in types.h:

/*
 * aligned_u64 should be used in defining kernel<->userspace ABIs to avoid
 * common 32/64-bit compat problems.
 * 64-bit values align to 4-byte boundaries on x86_32 (and possibly other
 * architectures) and to 8-byte boundaries on 64-bit architectures.  The new
 * aligned_64 type enforces 8-byte alignment so that structs containing
 * aligned_64 values have the same alignment on 32-bit and 64-bit architectures.
 * No conversions are necessary between 32-bit user-space and a 64-bit kernel.
 */
#define __aligned_u64 __u64 __attribute__((aligned(8)))

This poses a problem for our uapi wrappers: pointers in Go vary in size depending on the platform. We need a way to emulate __aligned_u64. So far, we've used a wrapper around unsafe.Pointer with a per-platform definition.

// on 64 bit platforms
type Pointer struct {
	ptr unsafe.Pointer
}

// on 32 bit little-endian platforms
type Pointer struct {
	ptr unsafe.Pointer
	pad uint32
}

This ensures that Pointer is always 64 bit, however it doesn't enforce that Pointer is aligned to 8 bytes (Go doesn't provide control over alignment). In addition, the generated uapi structs lack type information, the caller needs to know what to put where.

type ProgLoadAttr struct {
        ...
	FuncInfo           Pointer
	FuncInfoCnt        uint32
	LineInfoRecSize    uint32
	LineInfo           Pointer
        ...
}

Proposal: make sys.Pointer a generic type

When I originally came up with Pointer we didn't have generics available, so unsafe.Pointer was the simplest choice. We can do better now.

type Pointer[T any] struct { ... }

ProgLoadAttr would now become:

type ProgLoadAttr struct {
        ...
	FuncInfo           Pointer[FuncInfo]
        ...
}

We can do more neat things with this, for example introduce a pointer type for a C string:

type StringPointer Pointer[byte]

// NewStringPointer allocates a null-terminated backing slice for str and returns
// a pointer to it.
func NewStringPointer(str string) StringPointer {
	s, err := unix.ByteSliceFromString(str)
	if err != nil {
		return StringPointer{}
	}

	return StringPointer(SlicePointer(s))
}

This proposal requires that we'll start auto generating bpf_func_info, etc. via gentypes, like discussed).

Downsides

Using typed pointers makes it hard to support passing unsafe.Pointer to Map.Lookup. I think supporting that was always a bad idea, it makes the code quite complicated.

We'll also still need an "unsafe" escape hatch for some UAPI constructs. For example, bpf_link_create has a field iter_info that can point at several distinct types (the union bpf_iter_link_info). I've toyed around with using generics for this as well, but it's a much bigger change and I'm not sure it's worth it.

lmb avatar Jan 23 '23 18:01 lmb