Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit f0187f0b authored by Martin KaFai Lau's avatar Martin KaFai Lau Committed by Alexei Starovoitov
Browse files

bpf: libbpf: Refactor and bug fix on the bpf_func_info loading logic 



This patch refactor and fix a bug in the libbpf's bpf_func_info loading
logic.  The bug fix and refactoring are targeting the same
commit 2993e051 ("tools/bpf: add support to read .BTF.ext sections")
which is in the bpf-next branch.

1) In bpf_load_program_xattr(), it should retry when errno == E2BIG
   regardless of log_buf and log_buf_sz.  This patch fixes it.

2) btf_ext__reloc_init() and btf_ext__reloc() are essentially
   the same except btf_ext__reloc_init() always has insns_cnt == 0.
   Hence, btf_ext__reloc_init() is removed.

   btf_ext__reloc() is also renamed to btf_ext__reloc_func_info()
   to get ready for the line_info support in the next patch.

3) Consolidate func_info section logic from "btf_ext_parse_hdr()",
   "btf_ext_validate_func_info()" and "btf_ext__new()" to
   a new function "btf_ext_copy_func_info()" such that similar
   logic can be reused by the later libbpf's line_info patch.

4) The next line_info patch will store line_info_cnt instead of
   line_info_len in the bpf_program because the kernel is taking
   line_info_cnt also.  It will save a few "len" to "cnt" conversions
   and will also save some function args.

   Hence, this patch also makes bpf_program to store func_info_cnt
   instead of func_info_len.

5) btf_ext depends on btf.  e.g. the func_info's type_id
   in ".BTF.ext" is not useful when ".BTF" is absent.
   This patch only init the obj->btf_ext pointer after
   it has successfully init the obj->btf pointer.

   This can avoid always checking "obj->btf && obj->btf_ext"
   together for accessing ".BTF.ext".  Checking "obj->btf_ext"
   alone will do.

6) Move "struct btf_sec_func_info" from btf.h to btf.c.
   There is no external usage outside btf.c.

Fixes: 2993e051 ("tools/bpf: add support to read .BTF.ext sections")
Signed-off-by: default avatarMartin KaFai Lau <kafai@fb.com>
Acked-by: default avatarYonghong Song <yhs@fb.com>
Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
parent 4d6304c7

tools/lib/bpf/bpf.c

+5 −2
Original line number Diff line number Diff line
@@ -205,7 +205,7 @@ int bpf_load_program_xattr(const struct bpf_load_program_attr *load_attr, 
	       min(name_len, BPF_OBJ_NAME_LEN - 1));



	fd = sys_bpf(BPF_PROG_LOAD, &attr, sizeof(attr));

	if (fd >= 0 || !log_buf || !log_buf_sz)

	if (fd >= 0)

		return fd;



	/* After bpf_prog_load, the kernel may modify certain attributes
@@ -244,10 +244,13 @@ int bpf_load_program_xattr(const struct bpf_load_program_attr *load_attr, 


		fd = sys_bpf(BPF_PROG_LOAD, &attr, sizeof(attr));



		if (fd >= 0 || !log_buf || !log_buf_sz)

		if (fd >= 0)

			goto done;

	}



	if (!log_buf || !log_buf_sz)

		goto done;



	/* Try again with log */

	attr.log_buf = ptr_to_u64(log_buf);

	attr.log_size = log_buf_sz;

tools/lib/bpf/btf.c

+73 −118
Original line number Diff line number Diff line
@@ -43,6 +43,13 @@ struct btf_ext { 
	__u32 func_info_len;

};



struct btf_sec_func_info {

	__u32	sec_name_off;

	__u32	num_func_info;

	/* Followed by num_func_info number of bpf func_info records */

	__u8	data[0];

};



/* The minimum bpf_func_info checked by the loader */

struct bpf_func_info_min {

	__u32   insn_off;
@@ -479,41 +486,66 @@ int btf__get_from_id(__u32 id, struct btf **btf) 
	return err;

}



static int btf_ext_validate_func_info(const void *finfo, __u32 size,

static int btf_ext_copy_func_info(struct btf_ext *btf_ext,

				  __u8 *data, __u32 data_size,

				  btf_print_fn_t err_log)

{

	int sec_hdrlen = sizeof(struct btf_sec_func_info);

	__u32 size_left, num_records, record_size;

	const struct btf_ext_header *hdr = (struct btf_ext_header *)data;

	const struct btf_sec_func_info *sinfo;

	__u64 total_record_size;

	__u32 info_left, record_size;

	/* The start of the info sec (including the __u32 record_size). */

	const void *info;



	/* data and data_size do not include btf_ext_header from now on */

	data = data + hdr->hdr_len;

	data_size -= hdr->hdr_len;



	if (hdr->func_info_off & 0x03) {

		elog("BTF.ext func_info section is not aligned to 4 bytes\n");

		return -EINVAL;

	}



	if (data_size < hdr->func_info_off ||

	    hdr->func_info_len > data_size - hdr->func_info_off) {

		elog("func_info section (off:%u len:%u) is beyond the end of the ELF section .BTF.ext\n",

		     hdr->func_info_off, hdr->func_info_len);

		return -EINVAL;

	}



	info = data + hdr->func_info_off;

	info_left = hdr->func_info_len;



	/* At least a func_info record size */

	if (size < sizeof(__u32)) {

	if (info_left < sizeof(__u32)) {

		elog("BTF.ext func_info record size not found");

		return -EINVAL;

	}



	/* The record size needs to meet below minimum standard */

	record_size = *(__u32 *)finfo;

	/* The record size needs to meet the minimum standard */

	record_size = *(__u32 *)info;

	if (record_size < sizeof(struct bpf_func_info_min) ||

	    record_size % sizeof(__u32)) {

	    record_size & 0x03) {

		elog("BTF.ext func_info invalid record size");

		return -EINVAL;

	}



	sinfo = finfo + sizeof(__u32);

	size_left = size - sizeof(__u32);

	sinfo = info + sizeof(__u32);

	info_left -= sizeof(__u32);



	/* If no func_info records, return failure now so .BTF.ext

	 * won't be used.

	 */

	if (!size_left) {

	if (!info_left) {

		elog("BTF.ext no func info records");

		return -EINVAL;

	}



	while (size_left) {

		if (size_left < sec_hdrlen) {

	while (info_left) {

		unsigned int sec_hdrlen = sizeof(struct btf_sec_func_info);

		__u64 total_record_size;

		__u32 num_records;



		if (info_left < sec_hdrlen) {

			elog("BTF.ext func_info header not found");

			return -EINVAL;

		}
@@ -526,15 +558,23 @@ static int btf_ext_validate_func_info(const void *finfo, __u32 size, 


		total_record_size = sec_hdrlen +

				    (__u64)num_records * record_size;

		if (size_left < total_record_size) {

		if (info_left < total_record_size) {

			elog("incorrect BTF.ext num_func_info");

			return -EINVAL;

		}



		size_left -= total_record_size;

		info_left -= total_record_size;

		sinfo = (void *)sinfo + total_record_size;

	}



	btf_ext->func_info_len = hdr->func_info_len - sizeof(__u32);

	btf_ext->func_info_rec_size = record_size;

	btf_ext->func_info = malloc(btf_ext->func_info_len);

	if (!btf_ext->func_info)

		return -ENOMEM;

	memcpy(btf_ext->func_info, info + sizeof(__u32),

	       btf_ext->func_info_len);



	return 0;

}
@@ -542,8 +582,6 @@ static int btf_ext_parse_hdr(__u8 *data, __u32 data_size, 
			     btf_print_fn_t err_log)

{

	const struct btf_ext_header *hdr = (struct btf_ext_header *)data;

	__u32 meta_left, last_func_info_pos;

	void *finfo;



	if (data_size < offsetof(struct btf_ext_header, func_info_off) ||

	    data_size < hdr->hdr_len) {
@@ -566,34 +604,12 @@ static int btf_ext_parse_hdr(__u8 *data, __u32 data_size, 
		return -ENOTSUP;

	}



	meta_left = data_size - hdr->hdr_len;

	if (!meta_left) {

	if (data_size == hdr->hdr_len) {

		elog("BTF.ext has no data\n");

		return -EINVAL;

	}



	if (meta_left < hdr->func_info_off) {

		elog("Invalid BTF.ext func_info section offset:%u\n",

		     hdr->func_info_off);

		return -EINVAL;

	}



	if (hdr->func_info_off & 0x03) {

		elog("BTF.ext func_info section is not aligned to 4 bytes\n");

		return -EINVAL;

	}



	last_func_info_pos = hdr->hdr_len + hdr->func_info_off +

			     hdr->func_info_len;

	if (last_func_info_pos > data_size) {

		elog("Invalid BTF.ext func_info section size:%u\n",

		     hdr->func_info_len);

		return -EINVAL;

	}



	finfo = data + hdr->hdr_len + hdr->func_info_off;

	return btf_ext_validate_func_info(finfo, hdr->func_info_len,

					  err_log);

	return 0;

}



void btf_ext__free(struct btf_ext *btf_ext)
@@ -607,10 +623,7 @@ void btf_ext__free(struct btf_ext *btf_ext) 


struct btf_ext *btf_ext__new(__u8 *data, __u32 size, btf_print_fn_t err_log)

{

	const struct btf_ext_header *hdr;

	struct btf_ext *btf_ext;

	void *org_fdata, *fdata;

	__u32 hdrlen, size_u32;

	int err;



	err = btf_ext_parse_hdr(data, size, err_log);
@@ -621,81 +634,18 @@ struct btf_ext *btf_ext__new(__u8 *data, __u32 size, btf_print_fn_t err_log) 
	if (!btf_ext)

		return ERR_PTR(-ENOMEM);



	hdr = (const struct btf_ext_header *)data;

	hdrlen = hdr->hdr_len;

	size_u32 = sizeof(__u32);

	fdata = malloc(hdr->func_info_len - size_u32);

	if (!fdata) {

		free(btf_ext);

		return ERR_PTR(-ENOMEM);

	err = btf_ext_copy_func_info(btf_ext, data, size, err_log);

	if (err) {

		btf_ext__free(btf_ext);

		return ERR_PTR(err);

	}



	/* remember record size and copy rest of func_info data */

	org_fdata = data + hdrlen + hdr->func_info_off;

	btf_ext->func_info_rec_size = *(__u32 *)org_fdata;

	memcpy(fdata, org_fdata + size_u32, hdr->func_info_len - size_u32);

	btf_ext->func_info = fdata;

	btf_ext->func_info_len = hdr->func_info_len - size_u32;



	return btf_ext;

}



int btf_ext__reloc_init(struct btf *btf, struct btf_ext *btf_ext,

			const char *sec_name, void **func_info,

			__u32 *func_info_rec_size, __u32 *func_info_len)

{

	__u32 sec_hdrlen = sizeof(struct btf_sec_func_info);

	__u32 i, record_size, records_len;

	struct btf_sec_func_info *sinfo;

	const char *info_sec_name;

	__s64 remain_len;

	void *data;



	record_size = btf_ext->func_info_rec_size;

	sinfo = btf_ext->func_info;

	remain_len = btf_ext->func_info_len;



	while (remain_len > 0) {

		records_len = sinfo->num_func_info * record_size;

		info_sec_name = btf__name_by_offset(btf, sinfo->sec_name_off);

		if (strcmp(info_sec_name, sec_name)) {

			remain_len -= sec_hdrlen + records_len;

			sinfo = (void *)sinfo + sec_hdrlen + records_len;

			continue;

		}



		data = malloc(records_len);

		if (!data)

			return -ENOMEM;



		memcpy(data, sinfo->data, records_len);



		/* 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.

		 *

		 * adjust the insn offset only, the rest data will

		 * be passed to kernel.

		 */

		for (i = 0; i < sinfo->num_func_info; i++) {

			struct bpf_func_info_min *record;



			record = data + i * record_size;

			record->insn_off /= sizeof(struct bpf_insn);

		}



		*func_info = data;

		*func_info_len = records_len;

		*func_info_rec_size = record_size;

		return 0;

	}



	return -EINVAL;

}



int btf_ext__reloc(struct btf *btf, struct btf_ext *btf_ext,

int btf_ext__reloc_func_info(struct btf *btf, struct btf_ext *btf_ext,

			     const char *sec_name, __u32 insns_cnt,

		   void **func_info, __u32 *func_info_len)

			     void **func_info, __u32 *cnt)

{

	__u32 sec_hdrlen = sizeof(struct btf_sec_func_info);

	__u32 i, record_size, existing_flen, records_len;
@@ -716,7 +666,7 @@ int btf_ext__reloc(struct btf *btf, struct btf_ext *btf_ext, 
			continue;

		}



		existing_flen = *func_info_len;

		existing_flen = (*cnt) * record_size;

		data = realloc(*func_info, existing_flen + records_len);

		if (!data)

			return -ENOMEM;
@@ -734,9 +684,14 @@ int btf_ext__reloc(struct btf *btf, struct btf_ext *btf_ext, 
				insns_cnt;

		}

		*func_info = data;

		*func_info_len = existing_flen + records_len;

		*cnt += sinfo->num_func_info;

		return 0;

	}



	return -EINVAL;

	return -ENOENT;

}



__u32 btf_ext__func_info_rec_size(const struct btf_ext *btf_ext)

{

	return btf_ext->func_info_rec_size;

}

tools/lib/bpf/btf.h

+4 −13
Original line number Diff line number Diff line
@@ -53,13 +53,6 @@ struct btf_ext_header { 
	__u32	func_info_len;

};



struct btf_sec_func_info {

	__u32	sec_name_off;

	__u32	num_func_info;

	/* Followed by num_func_info number of bpf func_info records */

	__u8	data[0];

};



typedef int (*btf_print_fn_t)(const char *, ...)

	__attribute__((format(printf, 1, 2)));
@@ -77,12 +70,10 @@ LIBBPF_API int btf__get_from_id(__u32 id, struct btf **btf); 


struct btf_ext *btf_ext__new(__u8 *data, __u32 size, btf_print_fn_t err_log);

void btf_ext__free(struct btf_ext *btf_ext);

int btf_ext__reloc_init(struct btf *btf, struct btf_ext *btf_ext,

			const char *sec_name, void **func_info,

			__u32 *func_info_rec_size, __u32 *func_info_len);

int btf_ext__reloc(struct btf *btf, struct btf_ext *btf_ext,

		   const char *sec_name, __u32 insns_cnt, void **func_info,

		   __u32 *func_info_len);

int btf_ext__reloc_func_info(struct btf *btf, struct btf_ext *btf_ext,

			     const char *sec_name, __u32 insns_cnt,

			     void **func_info, __u32 *func_info_len);

__u32 btf_ext__func_info_rec_size(const struct btf_ext *btf_ext);



#ifdef __cplusplus

} /* extern "C" */

tools/lib/bpf/libbpf.c

+95 −44
Original line number Diff line number Diff line
@@ -167,7 +167,7 @@ struct bpf_program { 
	int btf_fd;

	void *func_info;

	__u32 func_info_rec_size;

	__u32 func_info_len;

	__u32 func_info_cnt;



	struct bpf_capabilities *caps;

};
@@ -779,6 +779,7 @@ static int bpf_object__elf_collect(struct bpf_object *obj, int flags) 
{

	Elf *elf = obj->efile.elf;

	GElf_Ehdr *ep = &obj->efile.ehdr;

	Elf_Data *btf_ext_data = NULL;

	Elf_Scn *scn = NULL;

	int idx = 0, err = 0;
@@ -841,14 +842,7 @@ static int bpf_object__elf_collect(struct bpf_object *obj, int flags) 
				obj->btf = NULL;

			}

		} else if (strcmp(name, BTF_EXT_ELF_SEC) == 0) {

			obj->btf_ext = btf_ext__new(data->d_buf, data->d_size,

						    __pr_debug);

			if (IS_ERR(obj->btf_ext)) {

				pr_warning("Error loading ELF section %s: %ld. Ignored and continue.\n",

					   BTF_EXT_ELF_SEC,

					   PTR_ERR(obj->btf_ext));

				obj->btf_ext = NULL;

			}

			btf_ext_data = data;

		} else if (sh.sh_type == SHT_SYMTAB) {

			if (obj->efile.symbols) {

				pr_warning("bpf: multiple SYMTAB in %s\n",
@@ -910,6 +904,22 @@ static int bpf_object__elf_collect(struct bpf_object *obj, int flags) 
		pr_warning("Corrupted ELF file: index of strtab invalid\n");

		return LIBBPF_ERRNO__FORMAT;

	}

	if (btf_ext_data) {

		if (!obj->btf) {

			pr_debug("Ignore ELF section %s because its depending ELF section %s is not found.\n",

				 BTF_EXT_ELF_SEC, BTF_ELF_SEC);

		} else {

			obj->btf_ext = btf_ext__new(btf_ext_data->d_buf,

						    btf_ext_data->d_size,

						    __pr_debug);

			if (IS_ERR(obj->btf_ext)) {

				pr_warning("Error loading ELF section %s: %ld. Ignored and continue.\n",

					   BTF_EXT_ELF_SEC,

					   PTR_ERR(obj->btf_ext));

				obj->btf_ext = NULL;

			}

		}

	}

	if (obj->efile.maps_shndx >= 0) {

		err = bpf_object__init_maps(obj, flags);

		if (err)
@@ -1275,6 +1285,69 @@ bpf_object__create_maps(struct bpf_object *obj) 
	return 0;

}



static int

check_btf_ext_reloc_err(struct bpf_program *prog, int err,

			void *btf_prog_info, const char *info_name)

{

	if (err != -ENOENT) {

		pr_warning("Error in loading %s for sec %s.\n",

			   info_name, prog->section_name);

		return err;

	}



	/* err == -ENOENT (i.e. prog->section_name not found in btf_ext) */



	if (btf_prog_info) {

		/*

		 * Some info has already been found but has problem

		 * in the last btf_ext reloc.  Must have to error

		 * out.

		 */

		pr_warning("Error in relocating %s for sec %s.\n",

			   info_name, prog->section_name);

		return err;

	}



	/*

	 * Have problem loading the very first info.  Ignore

	 * the rest.

	 */

	pr_warning("Cannot find %s for main program sec %s. Ignore all %s.\n",

		   info_name, prog->section_name, info_name);

	return 0;

}



static int

bpf_program_reloc_btf_ext(struct bpf_program *prog, struct bpf_object *obj,

			  const char *section_name,  __u32 insn_offset)

{

	int err;



	if (!insn_offset || prog->func_info) {

		/*

		 * !insn_offset => main program

		 *

		 * For sub prog, the main program's func_info has to

		 * be loaded first (i.e. prog->func_info != NULL)

		 */

		err = btf_ext__reloc_func_info(obj->btf, obj->btf_ext,

					       section_name, insn_offset,

					       &prog->func_info,

					       &prog->func_info_cnt);

		if (err)

			return check_btf_ext_reloc_err(prog, err,

						       prog->func_info,

						       "bpf_func_info");



		prog->func_info_rec_size = btf_ext__func_info_rec_size(obj->btf_ext);

	}



	if (!insn_offset)

		prog->btf_fd = btf__fd(obj->btf);



	return 0;

}



static int

bpf_program__reloc_text(struct bpf_program *prog, struct bpf_object *obj,

			struct reloc_desc *relo)
@@ -1306,18 +1379,13 @@ bpf_program__reloc_text(struct bpf_program *prog, struct bpf_object *obj, 
			return -ENOMEM;

		}



		if (obj->btf && obj->btf_ext) {

			err = btf_ext__reloc(obj->btf, obj->btf_ext,

		if (obj->btf_ext) {

			err = bpf_program_reloc_btf_ext(prog, obj,

							text->section_name,

					     prog->insns_cnt,

					     &prog->func_info,

					     &prog->func_info_len);

			if (err) {

				pr_warning("error in btf_ext__reloc for sec %s\n",

					   text->section_name);

							prog->insns_cnt);

			if (err)

				return err;

		}

		}



		memcpy(new_insn + prog->insns_cnt, text->insns,

		       text->insns_cnt * sizeof(*insn));
@@ -1341,19 +1409,12 @@ bpf_program__relocate(struct bpf_program *prog, struct bpf_object *obj) 
	if (!prog)

		return 0;



	if (obj->btf && obj->btf_ext) {

		err = btf_ext__reloc_init(obj->btf, obj->btf_ext,

					  prog->section_name,

					  &prog->func_info,

					  &prog->func_info_rec_size,

					  &prog->func_info_len);

		if (err) {

			pr_warning("err in btf_ext__reloc_init for sec %s\n",

				   prog->section_name);

	if (obj->btf_ext) {

		err = bpf_program_reloc_btf_ext(prog, obj,

						prog->section_name, 0);

		if (err)

			return err;

	}

		prog->btf_fd = btf__fd(obj->btf);

	}



	if (!prog->reloc_desc)

		return 0;
@@ -1444,8 +1505,7 @@ static int bpf_object__collect_reloc(struct bpf_object *obj) 


static int

load_program(struct bpf_program *prog, struct bpf_insn *insns, int insns_cnt,

	     char *license, __u32 kern_version, int *pfd,

	     __u32 func_info_cnt)

	     char *license, __u32 kern_version, int *pfd)

{

	struct bpf_load_program_attr load_attr;

	char *cp, errmsg[STRERR_BUFSIZE];
@@ -1465,8 +1525,7 @@ load_program(struct bpf_program *prog, struct bpf_insn *insns, int insns_cnt, 
	load_attr.prog_btf_fd = prog->btf_fd >= 0 ? prog->btf_fd : 0;

	load_attr.func_info = prog->func_info;

	load_attr.func_info_rec_size = prog->func_info_rec_size;

	load_attr.func_info_cnt = func_info_cnt;



	load_attr.func_info_cnt = prog->func_info_cnt;

	if (!load_attr.insns || !load_attr.insns_cnt)

		return -EINVAL;
@@ -1523,14 +1582,8 @@ int 
bpf_program__load(struct bpf_program *prog,

		  char *license, __u32 kern_version)

{

	__u32 func_info_cnt;

	int err = 0, fd, i;



	if (prog->func_info_len == 0)

		func_info_cnt = 0;

	else

		func_info_cnt = prog->func_info_len / prog->func_info_rec_size;



	if (prog->instances.nr < 0 || !prog->instances.fds) {

		if (prog->preprocessor) {

			pr_warning("Internal error: can't load program '%s'\n",
@@ -1553,8 +1606,7 @@ bpf_program__load(struct bpf_program *prog, 
				   prog->section_name, prog->instances.nr);

		}

		err = load_program(prog, prog->insns, prog->insns_cnt,

				   license, kern_version, &fd,

				   func_info_cnt);

				   license, kern_version, &fd);

		if (!err)

			prog->instances.fds[0] = fd;

		goto out;
@@ -1584,8 +1636,7 @@ bpf_program__load(struct bpf_program *prog, 


		err = load_program(prog, result.new_insn_ptr,

				   result.new_insn_cnt,

				   license, kern_version, &fd,

				   func_info_cnt);

				   license, kern_version, &fd);



		if (err) {

			pr_warning("Loading the %dth instance of program '%s' failed\n",