Merge branch 'bpf_func_info-improvements'

Martin KaFai Lau says: ==================== The patchset has a few improvements on bpf_func_info: 1. Improvements on the behaviors of info.func_info, info.func_info_cnt and info.func_info_rec_size. 2. Name change: s/insn_offset/insn_off/ Please see individual patch for details. ==================== Signed-off-by: Alexei Starovoitov <[email protected]>
ambv · Dec 6, 2018 · a06aef4 · a06aef4
2 parents 30da46b + 84ecc1f
commit a06aef4
Show file tree

Hide file tree

Showing 9 changed files with 54 additions and 47 deletions.
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
@@ -2991,7 +2991,7 @@ struct bpf_flow_keys {
 };
 
 struct bpf_func_info {
-	__u32	insn_offset;
+	__u32	insn_off;
 	__u32	type_id;
 };
 

diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
@@ -410,7 +410,7 @@ static void bpf_get_prog_name(const struct bpf_prog *prog, char *sym)
 	sym  = bin2hex(sym, prog->tag, sizeof(prog->tag));
 
 	/* prog->aux->name will be ignored if full btf name is available */
-	if (prog->aux->btf) {
+	if (prog->aux->func_info_cnt) {
 		type = btf_type_by_id(prog->aux->btf,
 				      prog->aux->func_info[prog->aux->func_idx].type_id);
 		func_name = btf_name_by_offset(prog->aux->btf, type->name_off);

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
@@ -2083,6 +2083,12 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
 				return -EFAULT;
 	}
 
+	if ((info.func_info_cnt || info.func_info_rec_size) &&
+	    info.func_info_rec_size != sizeof(struct bpf_func_info))
+		return -EINVAL;
+
+	info.func_info_rec_size = sizeof(struct bpf_func_info);
+
 	if (!capable(CAP_SYS_ADMIN)) {
 		info.jited_prog_len = 0;
 		info.xlated_prog_len = 0;
@@ -2226,35 +2232,23 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
 		}
 	}
 
-	if (prog->aux->btf) {
-		u32 krec_size = sizeof(struct bpf_func_info);
-		u32 ucnt, urec_size;
-
+	if (prog->aux->btf)
 		info.btf_id = btf_id(prog->aux->btf);
 
-		ucnt = info.func_info_cnt;
-		info.func_info_cnt = prog->aux->func_info_cnt;
-		urec_size = info.func_info_rec_size;
-		info.func_info_rec_size = krec_size;
-		if (ucnt) {
-			/* expect passed-in urec_size is what the kernel expects */
-			if (urec_size != info.func_info_rec_size)
-				return -EINVAL;
-
-			if (bpf_dump_raw_ok()) {
-				char __user *user_finfo;
-
-				user_finfo = u64_to_user_ptr(info.func_info);
-				ucnt = min_t(u32, info.func_info_cnt, ucnt);
-				if (copy_to_user(user_finfo, prog->aux->func_info,
-						 krec_size * ucnt))
-					return -EFAULT;
-			} else {
-				info.func_info_cnt = 0;
-			}
+	ulen = info.func_info_cnt;
+	info.func_info_cnt = prog->aux->func_info_cnt;
+	if (info.func_info_cnt && ulen) {
+		if (bpf_dump_raw_ok()) {
+			char __user *user_finfo;
+
+			user_finfo = u64_to_user_ptr(info.func_info);
+			ulen = min_t(u32, info.func_info_cnt, ulen);
+			if (copy_to_user(user_finfo, prog->aux->func_info,
+					 info.func_info_rec_size * ulen))
+				return -EFAULT;
+		} else {
+			info.func_info = 0;
 		}
-	} else {
-		info.func_info_cnt = 0;
 	}
 
 done:

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
@@ -4707,24 +4707,24 @@ static int check_btf_func(struct bpf_prog *prog, struct bpf_verifier_env *env,
 			goto free_btf;
 		}
 
-		/* check insn_offset */
+		/* check insn_off */
 		if (i == 0) {
-			if (krecord[i].insn_offset) {
+			if (krecord[i].insn_off) {
 				verbose(env,
-					"nonzero insn_offset %u for the first func info record",
-					krecord[i].insn_offset);
+					"nonzero insn_off %u for the first func info record",
+					krecord[i].insn_off);
 				ret = -EINVAL;
 				goto free_btf;
 			}
-		} else if (krecord[i].insn_offset <= prev_offset) {
+		} else if (krecord[i].insn_off <= prev_offset) {
 			verbose(env,
 				"same or smaller insn offset (%u) than previous func info record (%u)",
-				krecord[i].insn_offset, prev_offset);
+				krecord[i].insn_off, prev_offset);
 			ret = -EINVAL;
 			goto free_btf;
 		}
 
-		if (env->subprog_info[i].start != krecord[i].insn_offset) {
+		if (env->subprog_info[i].start != krecord[i].insn_off) {
 			verbose(env, "func_info BTF section doesn't match subprog layout in BPF program\n");
 			ret = -EINVAL;
 			goto free_btf;
@@ -4739,7 +4739,7 @@ static int check_btf_func(struct bpf_prog *prog, struct bpf_verifier_env *env,
 			goto free_btf;
 		}
 
-		prev_offset = krecord[i].insn_offset;
+		prev_offset = krecord[i].insn_off;
 		urecord += urec_size;
 	}
 
@@ -4762,7 +4762,7 @@ static void adjust_btf_func(struct bpf_verifier_env *env)
 		return;
 
 	for (i = 0; i < env->subprog_cnt; i++)
-		env->prog->aux->func_info[i].insn_offset = env->subprog_info[i].start;
+		env->prog->aux->func_info[i].insn_off = env->subprog_info[i].start;
 }
 
 /* check %cur's range satisfies %old's */

diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
@@ -589,6 +589,13 @@ static int do_dump(int argc, char **argv)
 		goto err_free;
 	}
 
+	if (func_info && !info.func_info) {
+		/* kernel.kptr_restrict is set.  No func_info available. */
+		free(func_info);
+		func_info = NULL;
+		finfo_cnt = 0;
+	}
+
 	if ((member_len == &info.jited_prog_len &&
 	     info.jited_prog_insns == 0) ||
 	    (member_len == &info.xlated_prog_len &&

diff --git a/tools/bpf/bpftool/xlated_dumper.c b/tools/bpf/bpftool/xlated_dumper.c
@@ -261,7 +261,7 @@ void dump_xlated_json(struct dump_data *dd, void *buf, unsigned int len,
 		jsonw_start_object(json_wtr);
 
 		if (btf && record) {
-			if (record->insn_offset == i) {
+			if (record->insn_off == i) {
 				btf_dumper_type_only(btf, record->type_id,
 						     func_sig,
 						     sizeof(func_sig));
@@ -330,7 +330,7 @@ void dump_xlated_plain(struct dump_data *dd, void *buf, unsigned int len,
 		}
 
 		if (btf && record) {
-			if (record->insn_offset == i) {
+			if (record->insn_off == i) {
 				btf_dumper_type_only(btf, record->type_id,
 						     func_sig,
 						     sizeof(func_sig));

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
@@ -2991,7 +2991,7 @@ struct bpf_flow_keys {
 };
 
 struct bpf_func_info {
-	__u32	insn_offset;
+	__u32	insn_off;
 	__u32	type_id;
 };
 

diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
@@ -45,7 +45,7 @@ struct btf_ext {
 
 /* The minimum bpf_func_info checked by the loader */
 struct bpf_func_info_min {
-	__u32   insn_offset;
+	__u32   insn_off;
 	__u32   type_id;
 };
 
@@ -670,7 +670,7 @@ int btf_ext__reloc_init(struct btf *btf, struct btf_ext *btf_ext,
 
 		memcpy(data, sinfo->data, records_len);
 
-		/* adjust the insn_offset, the data in .BTF.ext is
+		/* adjust the insn_off, the data in .BTF.ext is
 		 * the actual byte offset, and the kernel expects
 		 * the offset in term of bpf_insn.
 		 *
@@ -681,7 +681,7 @@ int btf_ext__reloc_init(struct btf *btf, struct btf_ext *btf_ext,
 			struct bpf_func_info_min *record;
 
 			record = data + i * record_size;
-			record->insn_offset /= sizeof(struct bpf_insn);
+			record->insn_off /= sizeof(struct bpf_insn);
 		}
 
 		*func_info = data;
@@ -722,15 +722,15 @@ int btf_ext__reloc(struct btf *btf, struct btf_ext *btf_ext,
 			return -ENOMEM;
 
 		memcpy(data + existing_flen, sinfo->data, records_len);
-		/* adjust insn_offset only, the rest data will be passed
+		/* adjust insn_off only, the rest data will be passed
 		 * to the kernel.
 		 */
 		for (i = 0; i < sinfo->num_func_info; i++) {
 			struct bpf_func_info_min *record;
 
 			record = data + existing_flen + i * record_size;
-			record->insn_offset =
-				record->insn_offset / sizeof(struct bpf_insn) +
+			record->insn_off =
+				record->insn_off / sizeof(struct bpf_insn) +
 				insns_cnt;
 		}
 		*func_info = data;

diff --git a/tools/testing/selftests/bpf/test_btf.c b/tools/testing/selftests/bpf/test_btf.c
@@ -3156,7 +3156,7 @@ static struct btf_func_type_test {
 },
 
 {
-	.descr = "func_type (Incorrect bpf_func_info.insn_offset)",
+	.descr = "func_type (Incorrect bpf_func_info.insn_off)",
 	.raw_types = {
 		BTF_TYPE_INT_ENC(NAME_TBD, BTF_INT_SIGNED, 0, 32, 4),	/* [1] */
 		BTF_TYPE_INT_ENC(NAME_TBD, 0, 0, 32, 4),	/* [2] */
@@ -3303,6 +3303,12 @@ static int do_test_func_type(int test_num)
 		goto done;
 	}
 
+	if (CHECK(!info.func_info,
+		  "info.func_info == 0. kernel.kptr_restrict is set?")) {
+		err = -1;
+		goto done;
+	}
+
 	finfo = func_info;
 	for (i = 0; i < 2; i++) {
 		if (CHECK(finfo->type_id != test->func_info[i][1],