On 7/2/26 9:06 PM, Anton Protopopov wrote:

On 26/07/02 04:36PM, Daniel Borkmann wrote:

quoted

The fd_array passed to BPF_PROG_LOAD carries the map and module BTF file
descriptors a program binds. The verifier reads it more than once during
a load: process_fd_array() walks it to bind the maps and BTFs, and
check_and_resolve_insns() and the kfunc BTF resolver later read it again
to resolve the program's BPF_PSEUDO_MAP_IDX and module kfunc refs.

For signed BPF, we need these upfront in memory, thus resolve each fd to
its object once and cache it by fd_array index, then bind that cached
object for the rest of the load. env->fd_array becomes a small per-slot
{map, btf} cache rather than a bpfptr_t; every later reference is then
an in-bounds lookup of an already-resolved object, and an index outside
the cache is rejected instead of read from user memory:

   - continuous (fd_array_cnt given): the caller declares the length and
     every entry is resolved and bound up front (used also by signed loader)

   - sparse (no fd_array_cnt): left as the legacy path with no fd_array
     cache; each reference reads its fd from the caller's fd_array and
     resolves it on the spot. Deduplication in used_maps and the kfunc BTF
     table keeps this correct, and only unsigned programs use this shape.

UAPI-wise nothing changes, its just the internals on how BPF processes
and caches the fd_array favors. Split these into separate helpers for
continuous versus sparse to make it easier to follow.

Overall looks ok. A few comments below.

[...]

quoted

-	/* Check for integer overflow */
-	if (attr->fd_array_cnt >= (U32_MAX / size)) {
-		verbose(env, "fd_array_cnt is too big (%u)\n", attr->fd_array_cnt);
-		return -EINVAL;
+	if (cnt > MAX_FD_ARRAY_CNT) {

So, I _think_ I've done the "unlimited" thing because there can be duplicates
in fd_array. The limit is actually tracked by __add_{map_btf} So here we hard
limit on the size of the fd_array itself. (Even without duplicates, this
fd_array can contain, say, MAX_KFUNC_BTFS different maps, which will be in any
case rejected by __add_used_map.)

quoted

+		verbose(env, "fd_array has too many entries (%u, max %u)\n",
+			cnt, MAX_FD_ARRAY_CNT);
+		return -E2BIG;
  	}
  

[...]

quoted

  
+static int process_fd_array(struct bpf_verifier_env *env,
+			    union bpf_attr *attr, bpfptr_t uattr)
+{
+	bpfptr_t fd_array = make_bpfptr(attr->fd_array, uattr.is_kernel);
+
+	if (bpfptr_is_null(fd_array))
+		return 0;
+	/*
+	 * New API: the caller passes fd_array_cnt and a continuous array that
+	 * is resolved and bound up front. Legacy API (no fd_array_cnt): keep
+	 * the caller's array and resolve entries lazily, on first reference.
+	 */
+	if (attr->fd_array_cnt)
+		return process_fd_array_continuous(env, fd_array,
+						   attr->fd_array_cnt);

Looks like this returns success in case (!fd_array && attr->fd_array_cnt),
which is a misconfiguration and should be rejected.

Thanks for the review, addressing both in v4!