[RFC PATCH bpf-next] bpf: Fix an error in verifying a field in a union

[Yafang Shao <laoar.shao@xxxxxxxxx>]

We are utilizing BPF LSM to monitor BPF operations within our container
environment. When we add support for raw_tracepoint, it hits below
error.

; (const void *)attr->raw_tracepoint.name);
27: (79) r3 = *(u64 *)(r2 +0)
access beyond the end of member map_type (mend:4) in struct (anon) with off 0 size 8

It can be reproduced with below BPF prog.

SEC("lsm/bpf")
int BPF_PROG(bpf_audit, int cmd, union bpf_attr *attr, unsigned int size)
{
	switch (cmd) {
	case BPF_RAW_TRACEPOINT_OPEN:
		bpf_printk("raw_tracepoint is %s", attr->raw_tracepoint.name);
		break;
	default:
		break;
	}
	return 0;
}

The reason is that when accessing a field in a union, such as bpf_attr, if
the field is located within a nested struct that is not the first member of
the union, it can result in incorrect field verification.

  union bpf_attr {
      struct {
          __u32 map_type; <<<< Actually it will find that field.
          __u32 key_size;
          __u32 value_size;
         ...
      };
      ...
      struct {
          __u64 name;    <<<< We want to verify this field.
          __u32 prog_fd;
      } raw_tracepoint;
  };

Considering the potential deep nesting levels, finding a perfect solution
to address this issue has proven challenging. Therefore, I propose a
solution where we simply skip the verification process if the field in
question is located within a union.

Signed-off-by: Yafang Shao <laoar.shao@xxxxxxxxx>
---
 kernel/bpf/btf.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index bd2cac057928..79ee4506bba4 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -6129,7 +6129,7 @@ enum bpf_struct_walk_result {
 static int btf_struct_walk(struct bpf_verifier_log *log, const struct btf *btf,
 			   const struct btf_type *t, int off, int size,
 			   u32 *next_btf_id, enum bpf_type_flag *flag,
-			   const char **field_name)
+			   const char **field_name, bool *in_union)
 {
 	u32 i, moff, mtrue_end, msize = 0, total_nelems = 0;
 	const struct btf_type *mtype, *elem_type = NULL;
@@ -6188,6 +6188,8 @@ static int btf_struct_walk(struct bpf_verifier_log *log, const struct btf *btf,
 		return -EACCES;
 	}
 
+	if (BTF_INFO_KIND(t->info) == BTF_KIND_UNION && !in_union)
+		*in_union = true;
 	for_each_member(i, t, member) {
 		/* offset of the field in bytes */
 		moff = __btf_member_bit_offset(t, member) / 8;
@@ -6372,7 +6374,7 @@ static int btf_struct_walk(struct bpf_verifier_log *log, const struct btf *btf,
 		 * that also allows using an array of int as a scratch
 		 * space. e.g. skb->cb[].
 		 */
-		if (off + size > mtrue_end) {
+		if (off + size > mtrue_end && !in_union) {
 			bpf_log(log,
 				"access beyond the end of member %s (mend:%u) in struct %s with off %u size %u\n",
 				mname, mtrue_end, tname, off, size);
@@ -6395,6 +6397,7 @@ int btf_struct_access(struct bpf_verifier_log *log,
 	enum bpf_type_flag tmp_flag = 0;
 	const struct btf_type *t;
 	u32 id = reg->btf_id;
+	bool in_union;
 	int err;
 
 	while (type_is_alloc(reg->type)) {
@@ -6421,7 +6424,8 @@ int btf_struct_access(struct bpf_verifier_log *log,
 
 	t = btf_type_by_id(btf, id);
 	do {
-		err = btf_struct_walk(log, btf, t, off, size, &id, &tmp_flag, field_name);
+		err = btf_struct_walk(log, btf, t, off, size, &id, &tmp_flag, field_name,
+				      &in_union);
 
 		switch (err) {
 		case WALK_PTR:
@@ -6481,6 +6485,7 @@ bool btf_struct_ids_match(struct bpf_verifier_log *log,
 {
 	const struct btf_type *type;
 	enum bpf_type_flag flag;
+	bool in_union;
 	int err;
 
 	/* Are we already done? */
@@ -6496,7 +6501,7 @@ bool btf_struct_ids_match(struct bpf_verifier_log *log,
 	type = btf_type_by_id(btf, id);
 	if (!type)
 		return false;
-	err = btf_struct_walk(log, btf, type, off, 1, &id, &flag, NULL);
+	err = btf_struct_walk(log, btf, type, off, 1, &id, &flag, NULL, &in_union);
 	if (err != WALK_STRUCT)
 		return false;
 
-- 
2.39.3