BPF List
 help / color / mirror / Atom feed
* [PATCH bpf-next v3 0/2] libbpf: Extend linker API to support in-memory ELF files
@ 2024-12-11 16:40 Alastair Robertson
  2024-12-11 16:40 ` [PATCH bpf-next v3 1/2] libbpf: Pull file-opening logic up to top-level functions Alastair Robertson
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Alastair Robertson @ 2024-12-11 16:40 UTC (permalink / raw)
  To: bpf, andrii; +Cc: Alastair Robertson

This gives API consumers the option of using anonymous files/memfds to
avoid writing temporary ELFs to disk, which will be useful for performing
linking as part of bpftrace's JIT compilation.

v3:
- Removed "filename" option. Now always generate our own filename for
  passed-in FDs and buffers.
- Use a common function (bpf_linker_add_file) for shared
  implementation of bpf_linker__add_file, bpf_linker__add_fd and
  bpf_linker__add_buf.

Alastair Robertson (2):
  libbpf: Pull file-opening logic up to top-level functions
  libbpf: Extend linker API to support in-memory ELF files

 tools/lib/bpf/libbpf.h   |   5 +
 tools/lib/bpf/libbpf.map |   4 +
 tools/lib/bpf/linker.c   | 228 ++++++++++++++++++++++++++++++---------
 3 files changed, 184 insertions(+), 53 deletions(-)

-- 
2.43.5


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH bpf-next v3 1/2] libbpf: Pull file-opening logic up to top-level functions
  2024-12-11 16:40 [PATCH bpf-next v3 0/2] libbpf: Extend linker API to support in-memory ELF files Alastair Robertson
@ 2024-12-11 16:40 ` Alastair Robertson
  2024-12-12 23:29   ` Andrii Nakryiko
  2024-12-11 16:40 ` [PATCH bpf-next v3 2/2] libbpf: Extend linker API to support in-memory ELF files Alastair Robertson
  2024-12-12 23:30 ` [PATCH bpf-next v3 0/2] " patchwork-bot+netdevbpf
  2 siblings, 1 reply; 6+ messages in thread
From: Alastair Robertson @ 2024-12-11 16:40 UTC (permalink / raw)
  To: bpf, andrii; +Cc: Alastair Robertson

Move the filename arguments and file-descriptor handling from
init_output_elf() and linker_load_obj_file() and instead handle them
at the top-level in bpf_linker__new() and bpf_linker__add_file().

This will allow the inner functions to be shared with a new,
non-filename-based, API in the next commit.

Signed-off-by: Alastair Robertson <ajor@meta.com>
---
 tools/lib/bpf/linker.c | 83 +++++++++++++++++++++---------------------
 1 file changed, 41 insertions(+), 42 deletions(-)

diff --git a/tools/lib/bpf/linker.c b/tools/lib/bpf/linker.c
index e56ba6e67451..eb2ac7afce01 100644
--- a/tools/lib/bpf/linker.c
+++ b/tools/lib/bpf/linker.c
@@ -157,10 +157,9 @@ struct bpf_linker {
 #define pr_warn_elf(fmt, ...)									\
 	libbpf_print(LIBBPF_WARN, "libbpf: " fmt ": %s\n", ##__VA_ARGS__, elf_errmsg(-1))
 
-static int init_output_elf(struct bpf_linker *linker, const char *file);
+static int init_output_elf(struct bpf_linker *linker);
 
-static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
-				const struct bpf_linker_file_opts *opts,
+static int linker_load_obj_file(struct bpf_linker *linker,
 				struct src_obj *obj);
 static int linker_sanity_check_elf(struct src_obj *obj);
 static int linker_sanity_check_elf_symtab(struct src_obj *obj, struct src_sec *sec);
@@ -233,9 +232,20 @@ struct bpf_linker *bpf_linker__new(const char *filename, struct bpf_linker_opts
 	if (!linker)
 		return errno = ENOMEM, NULL;
 
-	linker->fd = -1;
+	linker->filename = strdup(filename);
+	if (!linker->filename) {
+		err = -ENOMEM;
+		goto err_out;
+	}
+
+	linker->fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC, 0644);
+	if (linker->fd < 0) {
+		err = -errno;
+		pr_warn("failed to create '%s': %d\n", filename, err);
+		goto err_out;
+	}
 
-	err = init_output_elf(linker, filename);
+	err = init_output_elf(linker);
 	if (err)
 		goto err_out;
 
@@ -294,23 +304,12 @@ static Elf64_Sym *add_new_sym(struct bpf_linker *linker, size_t *sym_idx)
 	return sym;
 }
 
-static int init_output_elf(struct bpf_linker *linker, const char *file)
+static int init_output_elf(struct bpf_linker *linker)
 {
 	int err, str_off;
 	Elf64_Sym *init_sym;
 	struct dst_sec *sec;
 
-	linker->filename = strdup(file);
-	if (!linker->filename)
-		return -ENOMEM;
-
-	linker->fd = open(file, O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC, 0644);
-	if (linker->fd < 0) {
-		err = -errno;
-		pr_warn("failed to create '%s': %s\n", file, errstr(err));
-		return err;
-	}
-
 	linker->elf = elf_begin(linker->fd, ELF_C_WRITE, NULL);
 	if (!linker->elf) {
 		pr_warn_elf("failed to create ELF object");
@@ -440,7 +439,7 @@ int bpf_linker__add_file(struct bpf_linker *linker, const char *filename,
 			 const struct bpf_linker_file_opts *opts)
 {
 	struct src_obj obj = {};
-	int err = 0;
+	int err = 0, fd;
 
 	if (!OPTS_VALID(opts, bpf_linker_file_opts))
 		return libbpf_err(-EINVAL);
@@ -448,7 +447,16 @@ int bpf_linker__add_file(struct bpf_linker *linker, const char *filename,
 	if (!linker->elf)
 		return libbpf_err(-EINVAL);
 
-	err = err ?: linker_load_obj_file(linker, filename, opts, &obj);
+	fd = open(filename, O_RDONLY | O_CLOEXEC);
+	if (fd < 0) {
+		pr_warn("failed to open file '%s': %s\n", filename, errstr(errno));
+		return -errno;
+	}
+
+	obj.filename = filename;
+	obj.fd = fd;
+
+	err = err ?: linker_load_obj_file(linker, &obj);
 	err = err ?: linker_append_sec_data(linker, &obj);
 	err = err ?: linker_append_elf_syms(linker, &obj);
 	err = err ?: linker_append_elf_relos(linker, &obj);
@@ -534,8 +542,7 @@ static struct src_sec *add_src_sec(struct src_obj *obj, const char *sec_name)
 	return sec;
 }
 
-static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
-				const struct bpf_linker_file_opts *opts,
+static int linker_load_obj_file(struct bpf_linker *linker,
 				struct src_obj *obj)
 {
 	int err = 0;
@@ -554,26 +561,18 @@ static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
 #error "Unknown __BYTE_ORDER__"
 #endif
 
-	pr_debug("linker: adding object file '%s'...\n", filename);
-
-	obj->filename = filename;
+	pr_debug("linker: adding object file '%s'...\n", obj->filename);
 
-	obj->fd = open(filename, O_RDONLY | O_CLOEXEC);
-	if (obj->fd < 0) {
-		err = -errno;
-		pr_warn("failed to open file '%s': %s\n", filename, errstr(err));
-		return err;
-	}
 	obj->elf = elf_begin(obj->fd, ELF_C_READ_MMAP, NULL);
 	if (!obj->elf) {
-		pr_warn_elf("failed to parse ELF file '%s'", filename);
+		pr_warn_elf("failed to parse ELF file '%s'", obj->filename);
 		return -EINVAL;
 	}
 
 	/* Sanity check ELF file high-level properties */
 	ehdr = elf64_getehdr(obj->elf);
 	if (!ehdr) {
-		pr_warn_elf("failed to get ELF header for %s", filename);
+		pr_warn_elf("failed to get ELF header for %s", obj->filename);
 		return -EINVAL;
 	}
 
@@ -581,7 +580,7 @@ static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
 	obj_byteorder = ehdr->e_ident[EI_DATA];
 	if (obj_byteorder != ELFDATA2LSB && obj_byteorder != ELFDATA2MSB) {
 		err = -EOPNOTSUPP;
-		pr_warn("unknown byte order of ELF file %s\n", filename);
+		pr_warn("unknown byte order of ELF file %s\n", obj->filename);
 		return err;
 	}
 	if (link_byteorder == ELFDATANONE) {
@@ -591,7 +590,7 @@ static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
 			 obj_byteorder == ELFDATA2MSB ? "big" : "little");
 	} else if (link_byteorder != obj_byteorder) {
 		err = -EOPNOTSUPP;
-		pr_warn("byte order mismatch with ELF file %s\n", filename);
+		pr_warn("byte order mismatch with ELF file %s\n", obj->filename);
 		return err;
 	}
 
@@ -599,12 +598,12 @@ static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
 	    || ehdr->e_machine != EM_BPF
 	    || ehdr->e_ident[EI_CLASS] != ELFCLASS64) {
 		err = -EOPNOTSUPP;
-		pr_warn_elf("unsupported kind of ELF file %s", filename);
+		pr_warn_elf("unsupported kind of ELF file %s", obj->filename);
 		return err;
 	}
 
 	if (elf_getshdrstrndx(obj->elf, &obj->shstrs_sec_idx)) {
-		pr_warn_elf("failed to get SHSTRTAB section index for %s", filename);
+		pr_warn_elf("failed to get SHSTRTAB section index for %s", obj->filename);
 		return -EINVAL;
 	}
 
@@ -616,21 +615,21 @@ static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
 		shdr = elf64_getshdr(scn);
 		if (!shdr) {
 			pr_warn_elf("failed to get section #%zu header for %s",
-				    sec_idx, filename);
+				    sec_idx, obj->filename);
 			return -EINVAL;
 		}
 
 		sec_name = elf_strptr(obj->elf, obj->shstrs_sec_idx, shdr->sh_name);
 		if (!sec_name) {
 			pr_warn_elf("failed to get section #%zu name for %s",
-				    sec_idx, filename);
+				    sec_idx, obj->filename);
 			return -EINVAL;
 		}
 
 		data = elf_getdata(scn, 0);
 		if (!data) {
 			pr_warn_elf("failed to get section #%zu (%s) data from %s",
-				    sec_idx, sec_name, filename);
+				    sec_idx, sec_name, obj->filename);
 			return -EINVAL;
 		}
 
@@ -666,7 +665,7 @@ static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
 				err = libbpf_get_error(obj->btf);
 				if (err) {
 					pr_warn("failed to parse .BTF from %s: %s\n",
-						filename, errstr(err));
+						obj->filename, errstr(err));
 					return err;
 				}
 				sec->skipped = true;
@@ -677,7 +676,7 @@ static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
 				err = libbpf_get_error(obj->btf_ext);
 				if (err) {
 					pr_warn("failed to parse .BTF.ext from '%s': %s\n",
-						filename, errstr(err));
+						obj->filename, errstr(err));
 					return err;
 				}
 				sec->skipped = true;
@@ -694,7 +693,7 @@ static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
 			break;
 		default:
 			pr_warn("unrecognized section #%zu (%s) in %s\n",
-				sec_idx, sec_name, filename);
+				sec_idx, sec_name, obj->filename);
 			err = -EINVAL;
 			return err;
 		}
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH bpf-next v3 2/2] libbpf: Extend linker API to support in-memory ELF files
  2024-12-11 16:40 [PATCH bpf-next v3 0/2] libbpf: Extend linker API to support in-memory ELF files Alastair Robertson
  2024-12-11 16:40 ` [PATCH bpf-next v3 1/2] libbpf: Pull file-opening logic up to top-level functions Alastair Robertson
@ 2024-12-11 16:40 ` Alastair Robertson
  2024-12-12 23:29   ` Andrii Nakryiko
  2024-12-12 23:30 ` [PATCH bpf-next v3 0/2] " patchwork-bot+netdevbpf
  2 siblings, 1 reply; 6+ messages in thread
From: Alastair Robertson @ 2024-12-11 16:40 UTC (permalink / raw)
  To: bpf, andrii; +Cc: Alastair Robertson

The new_fd and add_fd functions correspond to the original new and
add_file functions, but accept an FD instead of a file name. This
gives API consumers the option of using anonymous files/memfds to
avoid writing ELFs to disk.

This new API will be useful for performing linking as part of
bpftrace's JIT compilation.

The add_buf function is a convenience wrapper that does the work of
creating a memfd for the caller.

Signed-off-by: Alastair Robertson <ajor@meta.com>
---
 tools/lib/bpf/libbpf.h   |   5 ++
 tools/lib/bpf/libbpf.map |   4 +
 tools/lib/bpf/linker.c   | 163 ++++++++++++++++++++++++++++++++++-----
 3 files changed, 152 insertions(+), 20 deletions(-)

diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index b2ce3a72b11d..d45807103565 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -1796,9 +1796,14 @@ struct bpf_linker_file_opts {
 struct bpf_linker;
 
 LIBBPF_API struct bpf_linker *bpf_linker__new(const char *filename, struct bpf_linker_opts *opts);
+LIBBPF_API struct bpf_linker *bpf_linker__new_fd(int fd, struct bpf_linker_opts *opts);
 LIBBPF_API int bpf_linker__add_file(struct bpf_linker *linker,
 				    const char *filename,
 				    const struct bpf_linker_file_opts *opts);
+LIBBPF_API int bpf_linker__add_fd(struct bpf_linker *linker, int fd,
+				  const struct bpf_linker_file_opts *opts);
+LIBBPF_API int bpf_linker__add_buf(struct bpf_linker *linker, void *buf, size_t buf_sz,
+				   const struct bpf_linker_file_opts *opts);
 LIBBPF_API int bpf_linker__finalize(struct bpf_linker *linker);
 LIBBPF_API void bpf_linker__free(struct bpf_linker *linker);
 
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 54b6f312cfa8..a8b2936a1646 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -432,4 +432,8 @@ LIBBPF_1.5.0 {
 } LIBBPF_1.4.0;
 
 LIBBPF_1.6.0 {
+	global:
+		bpf_linker__add_buf;
+		bpf_linker__add_fd;
+		bpf_linker__new_fd;
 } LIBBPF_1.5.0;
diff --git a/tools/lib/bpf/linker.c b/tools/lib/bpf/linker.c
index eb2ac7afce01..6b3b86d98f6c 100644
--- a/tools/lib/bpf/linker.c
+++ b/tools/lib/bpf/linker.c
@@ -4,6 +4,10 @@
  *
  * Copyright (c) 2021 Facebook
  */
+#ifndef _GNU_SOURCE
+#define _GNU_SOURCE
+#endif
+
 #include <stdbool.h>
 #include <stddef.h>
 #include <stdio.h>
@@ -16,6 +20,7 @@
 #include <elf.h>
 #include <libelf.h>
 #include <fcntl.h>
+#include <sys/mman.h>
 #include "libbpf.h"
 #include "btf.h"
 #include "libbpf_internal.h"
@@ -152,6 +157,8 @@ struct bpf_linker {
 	/* global (including extern) ELF symbols */
 	int glob_sym_cnt;
 	struct glob_sym *glob_syms;
+
+	bool fd_is_owned;
 };
 
 #define pr_warn_elf(fmt, ...)									\
@@ -159,6 +166,9 @@ struct bpf_linker {
 
 static int init_output_elf(struct bpf_linker *linker);
 
+static int bpf_linker_add_file(struct bpf_linker *linker, int fd,
+			       const char *filename);
+
 static int linker_load_obj_file(struct bpf_linker *linker,
 				struct src_obj *obj);
 static int linker_sanity_check_elf(struct src_obj *obj);
@@ -190,7 +200,7 @@ void bpf_linker__free(struct bpf_linker *linker)
 	if (linker->elf)
 		elf_end(linker->elf);
 
-	if (linker->fd >= 0)
+	if (linker->fd >= 0 && linker->fd_is_owned)
 		close(linker->fd);
 
 	strset__free(linker->strtab_strs);
@@ -244,6 +254,49 @@ struct bpf_linker *bpf_linker__new(const char *filename, struct bpf_linker_opts
 		pr_warn("failed to create '%s': %d\n", filename, err);
 		goto err_out;
 	}
+	linker->fd_is_owned = true;
+
+	err = init_output_elf(linker);
+	if (err)
+		goto err_out;
+
+	return linker;
+
+err_out:
+	bpf_linker__free(linker);
+	return errno = -err, NULL;
+}
+
+struct bpf_linker *bpf_linker__new_fd(int fd, struct bpf_linker_opts *opts)
+{
+	struct bpf_linker *linker;
+	char filename[32];
+	int err;
+
+	if (fd < 0)
+		return errno = EINVAL, NULL;
+
+	if (!OPTS_VALID(opts, bpf_linker_opts))
+		return errno = EINVAL, NULL;
+
+	if (elf_version(EV_CURRENT) == EV_NONE) {
+		pr_warn_elf("libelf initialization failed");
+		return errno = EINVAL, NULL;
+	}
+
+	linker = calloc(1, sizeof(*linker));
+	if (!linker)
+		return errno = ENOMEM, NULL;
+
+	snprintf(filename, sizeof(filename), "fd:%d", fd);
+	linker->filename = strdup(filename);
+	if (!linker->filename) {
+		err = -ENOMEM;
+		goto err_out;
+	}
+
+	linker->fd = fd;
+	linker->fd_is_owned = false;
 
 	err = init_output_elf(linker);
 	if (err)
@@ -435,23 +488,11 @@ static int init_output_elf(struct bpf_linker *linker)
 	return 0;
 }
 
-int bpf_linker__add_file(struct bpf_linker *linker, const char *filename,
-			 const struct bpf_linker_file_opts *opts)
+static int bpf_linker_add_file(struct bpf_linker *linker, int fd,
+			       const char *filename)
 {
 	struct src_obj obj = {};
-	int err = 0, fd;
-
-	if (!OPTS_VALID(opts, bpf_linker_file_opts))
-		return libbpf_err(-EINVAL);
-
-	if (!linker->elf)
-		return libbpf_err(-EINVAL);
-
-	fd = open(filename, O_RDONLY | O_CLOEXEC);
-	if (fd < 0) {
-		pr_warn("failed to open file '%s': %s\n", filename, errstr(errno));
-		return -errno;
-	}
+	int err = 0;
 
 	obj.filename = filename;
 	obj.fd = fd;
@@ -471,12 +512,93 @@ int bpf_linker__add_file(struct bpf_linker *linker, const char *filename,
 	free(obj.sym_map);
 	if (obj.elf)
 		elf_end(obj.elf);
-	if (obj.fd >= 0)
-		close(obj.fd);
 
 	return libbpf_err(err);
 }
 
+int bpf_linker__add_file(struct bpf_linker *linker, const char *filename,
+			 const struct bpf_linker_file_opts *opts)
+{
+	int fd, ret;
+
+	if (!OPTS_VALID(opts, bpf_linker_file_opts))
+		return libbpf_err(-EINVAL);
+
+	if (!linker->elf)
+		return libbpf_err(-EINVAL);
+
+	fd = open(filename, O_RDONLY | O_CLOEXEC);
+	if (fd < 0) {
+		pr_warn("failed to open file '%s': %s\n", filename, errstr(errno));
+		return -errno;
+	}
+
+	ret = bpf_linker_add_file(linker, fd, filename);
+
+	close(fd);
+
+	return ret;
+}
+
+int bpf_linker__add_fd(struct bpf_linker *linker, int fd,
+		       const struct bpf_linker_file_opts *opts)
+{
+	char filename[32];
+	int ret;
+
+	if (!OPTS_VALID(opts, bpf_linker_file_opts))
+		return libbpf_err(-EINVAL);
+
+	if (!linker->elf)
+		return libbpf_err(-EINVAL);
+
+	if (fd < 0)
+		return libbpf_err(-EINVAL);
+
+	snprintf(filename, sizeof(filename), "fd:%d", fd);
+
+	ret = bpf_linker_add_file(linker, fd, filename);
+
+	return ret;
+}
+
+int bpf_linker__add_buf(struct bpf_linker *linker, void *buf, size_t buf_sz,
+			const struct bpf_linker_file_opts *opts)
+{
+	char filename[32];
+	int fd, written, ret;
+
+	if (!OPTS_VALID(opts, bpf_linker_file_opts))
+		return libbpf_err(-EINVAL);
+
+	if (!linker->elf)
+		return libbpf_err(-EINVAL);
+
+	snprintf(filename, sizeof(filename), "mem:%p+%zu", buf, buf_sz);
+
+	fd = memfd_create(filename, 0);
+	if (fd < 0) {
+		pr_warn("failed to create memfd '%s': %s\n", filename, errstr(errno));
+		return -errno;
+	}
+
+	written = 0;
+	while (written < buf_sz) {
+		ret = write(fd, buf, buf_sz);
+		if (ret < 0) {
+			pr_warn("failed to write '%s': %s\n", filename, errstr(errno));
+			return -errno;
+		}
+		written += ret;
+	}
+
+	ret = bpf_linker_add_file(linker, fd, filename);
+
+	close(fd);
+
+	return ret;
+}
+
 static bool is_dwarf_sec_name(const char *name)
 {
 	/* approximation, but the actual list is too long */
@@ -2686,9 +2808,10 @@ int bpf_linker__finalize(struct bpf_linker *linker)
 	}
 
 	elf_end(linker->elf);
-	close(linker->fd);
-
 	linker->elf = NULL;
+
+	if (linker->fd_is_owned)
+		close(linker->fd);
 	linker->fd = -1;
 
 	return 0;
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf-next v3 1/2] libbpf: Pull file-opening logic up to top-level functions
  2024-12-11 16:40 ` [PATCH bpf-next v3 1/2] libbpf: Pull file-opening logic up to top-level functions Alastair Robertson
@ 2024-12-12 23:29   ` Andrii Nakryiko
  0 siblings, 0 replies; 6+ messages in thread
From: Andrii Nakryiko @ 2024-12-12 23:29 UTC (permalink / raw)
  To: Alastair Robertson; +Cc: bpf, andrii

On Wed, Dec 11, 2024 at 8:40 AM Alastair Robertson <ajor@meta.com> wrote:
>
> Move the filename arguments and file-descriptor handling from
> init_output_elf() and linker_load_obj_file() and instead handle them
> at the top-level in bpf_linker__new() and bpf_linker__add_file().
>
> This will allow the inner functions to be shared with a new,
> non-filename-based, API in the next commit.
>
> Signed-off-by: Alastair Robertson <ajor@meta.com>
> ---
>  tools/lib/bpf/linker.c | 83 +++++++++++++++++++++---------------------
>  1 file changed, 41 insertions(+), 42 deletions(-)
>

[...]

> @@ -440,7 +439,7 @@ int bpf_linker__add_file(struct bpf_linker *linker, const char *filename,
>                          const struct bpf_linker_file_opts *opts)
>  {
>         struct src_obj obj = {};
> -       int err = 0;
> +       int err = 0, fd;
>
>         if (!OPTS_VALID(opts, bpf_linker_file_opts))
>                 return libbpf_err(-EINVAL);
> @@ -448,7 +447,16 @@ int bpf_linker__add_file(struct bpf_linker *linker, const char *filename,
>         if (!linker->elf)
>                 return libbpf_err(-EINVAL);
>
> -       err = err ?: linker_load_obj_file(linker, filename, opts, &obj);
> +       fd = open(filename, O_RDONLY | O_CLOEXEC);
> +       if (fd < 0) {
> +               pr_warn("failed to open file '%s': %s\n", filename, errstr(errno));
> +               return -errno;

errno can be clobbered by pr_warn(), so need to save it like in
another pr_warn case above, fixed while applying

> +       }
> +
> +       obj.filename = filename;
> +       obj.fd = fd;
> +
> +       err = err ?: linker_load_obj_file(linker, &obj);
>         err = err ?: linker_append_sec_data(linker, &obj);
>         err = err ?: linker_append_elf_syms(linker, &obj);
>         err = err ?: linker_append_elf_relos(linker, &obj);
> @@ -534,8 +542,7 @@ static struct src_sec *add_src_sec(struct src_obj *obj, const char *sec_name)
>         return sec;
>  }
>
> -static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
> -                               const struct bpf_linker_file_opts *opts,
> +static int linker_load_obj_file(struct bpf_linker *linker,
>                                 struct src_obj *obj)
>  {
>         int err = 0;

[...]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf-next v3 2/2] libbpf: Extend linker API to support in-memory ELF files
  2024-12-11 16:40 ` [PATCH bpf-next v3 2/2] libbpf: Extend linker API to support in-memory ELF files Alastair Robertson
@ 2024-12-12 23:29   ` Andrii Nakryiko
  0 siblings, 0 replies; 6+ messages in thread
From: Andrii Nakryiko @ 2024-12-12 23:29 UTC (permalink / raw)
  To: Alastair Robertson; +Cc: bpf, andrii

On Wed, Dec 11, 2024 at 8:40 AM Alastair Robertson <ajor@meta.com> wrote:
>
> The new_fd and add_fd functions correspond to the original new and
> add_file functions, but accept an FD instead of a file name. This
> gives API consumers the option of using anonymous files/memfds to
> avoid writing ELFs to disk.
>
> This new API will be useful for performing linking as part of
> bpftrace's JIT compilation.
>
> The add_buf function is a convenience wrapper that does the work of
> creating a memfd for the caller.
>
> Signed-off-by: Alastair Robertson <ajor@meta.com>
> ---
>  tools/lib/bpf/libbpf.h   |   5 ++
>  tools/lib/bpf/libbpf.map |   4 +
>  tools/lib/bpf/linker.c   | 163 ++++++++++++++++++++++++++++++++++-----
>  3 files changed, 152 insertions(+), 20 deletions(-)
>

There were a bunch of errno mishandling issues and a small memory
leak. I fixed all that up and landed in bpf-next.

> +int bpf_linker__add_file(struct bpf_linker *linker, const char *filename,
> +                        const struct bpf_linker_file_opts *opts)
> +{
> +       int fd, ret;
> +
> +       if (!OPTS_VALID(opts, bpf_linker_file_opts))
> +               return libbpf_err(-EINVAL);
> +
> +       if (!linker->elf)
> +               return libbpf_err(-EINVAL);
> +
> +       fd = open(filename, O_RDONLY | O_CLOEXEC);
> +       if (fd < 0) {
> +               pr_warn("failed to open file '%s': %s\n", filename, errstr(errno));
> +               return -errno;

same issue with errno clobbering, fixed, but please be careful with
errno, pretty much anything can clobber errno in libc, except free()
and maybe a few more APIs

> +       }
> +
> +       ret = bpf_linker_add_file(linker, fd, filename);
> +
> +       close(fd);
> +
> +       return ret;

this needed to be libbpf_err(ret), because close() can clobber errno,
added while applying

> +}
> +
> +int bpf_linker__add_fd(struct bpf_linker *linker, int fd,
> +                      const struct bpf_linker_file_opts *opts)
> +{
> +       char filename[32];
> +       int ret;
> +
> +       if (!OPTS_VALID(opts, bpf_linker_file_opts))
> +               return libbpf_err(-EINVAL);
> +
> +       if (!linker->elf)
> +               return libbpf_err(-EINVAL);
> +
> +       if (fd < 0)
> +               return libbpf_err(-EINVAL);
> +
> +       snprintf(filename, sizeof(filename), "fd:%d", fd);
> +
> +       ret = bpf_linker_add_file(linker, fd, filename);
> +
> +       return ret;

here as well

> +}
> +
> +int bpf_linker__add_buf(struct bpf_linker *linker, void *buf, size_t buf_sz,
> +                       const struct bpf_linker_file_opts *opts)
> +{
> +       char filename[32];
> +       int fd, written, ret;
> +
> +       if (!OPTS_VALID(opts, bpf_linker_file_opts))
> +               return libbpf_err(-EINVAL);
> +
> +       if (!linker->elf)
> +               return libbpf_err(-EINVAL);
> +
> +       snprintf(filename, sizeof(filename), "mem:%p+%zu", buf, buf_sz);
> +
> +       fd = memfd_create(filename, 0);
> +       if (fd < 0) {

and here

> +               pr_warn("failed to create memfd '%s': %s\n", filename, errstr(errno));
> +               return -errno;
> +       }
> +
> +       written = 0;
> +       while (written < buf_sz) {
> +               ret = write(fd, buf, buf_sz);
> +               if (ret < 0) {

and here

and also you were leaking memfd here, I added jump to close(fd)

> +                       pr_warn("failed to write '%s': %s\n", filename, errstr(errno));
> +                       return -errno;
> +               }
> +               written += ret;
> +       }
> +
> +       ret = bpf_linker_add_file(linker, fd, filename);
> +
> +       close(fd);
> +
> +       return ret;

and here


> +}
> +
>  static bool is_dwarf_sec_name(const char *name)
>  {
>         /* approximation, but the actual list is too long */
> @@ -2686,9 +2808,10 @@ int bpf_linker__finalize(struct bpf_linker *linker)
>         }
>
>         elf_end(linker->elf);
> -       close(linker->fd);
> -
>         linker->elf = NULL;
> +
> +       if (linker->fd_is_owned)
> +               close(linker->fd);
>         linker->fd = -1;
>
>         return 0;
> --
> 2.43.5
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf-next v3 0/2] libbpf: Extend linker API to support in-memory ELF files
  2024-12-11 16:40 [PATCH bpf-next v3 0/2] libbpf: Extend linker API to support in-memory ELF files Alastair Robertson
  2024-12-11 16:40 ` [PATCH bpf-next v3 1/2] libbpf: Pull file-opening logic up to top-level functions Alastair Robertson
  2024-12-11 16:40 ` [PATCH bpf-next v3 2/2] libbpf: Extend linker API to support in-memory ELF files Alastair Robertson
@ 2024-12-12 23:30 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-12-12 23:30 UTC (permalink / raw)
  To: Alastair Robertson; +Cc: bpf, andrii

Hello:

This series was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:

On Wed, 11 Dec 2024 08:40:28 -0800 you wrote:
> This gives API consumers the option of using anonymous files/memfds to
> avoid writing temporary ELFs to disk, which will be useful for performing
> linking as part of bpftrace's JIT compilation.
> 
> v3:
> - Removed "filename" option. Now always generate our own filename for
>   passed-in FDs and buffers.
> - Use a common function (bpf_linker_add_file) for shared
>   implementation of bpf_linker__add_file, bpf_linker__add_fd and
>   bpf_linker__add_buf.
> 
> [...]

Here is the summary with links:
  - [bpf-next,v3,1/2] libbpf: Pull file-opening logic up to top-level functions
    https://git.kernel.org/bpf/bpf-next/c/b641712925bf
  - [bpf-next,v3,2/2] libbpf: Extend linker API to support in-memory ELF files
    https://git.kernel.org/bpf/bpf-next/c/6d5e5e5d7ce1

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-12-12 23:30 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-11 16:40 [PATCH bpf-next v3 0/2] libbpf: Extend linker API to support in-memory ELF files Alastair Robertson
2024-12-11 16:40 ` [PATCH bpf-next v3 1/2] libbpf: Pull file-opening logic up to top-level functions Alastair Robertson
2024-12-12 23:29   ` Andrii Nakryiko
2024-12-11 16:40 ` [PATCH bpf-next v3 2/2] libbpf: Extend linker API to support in-memory ELF files Alastair Robertson
2024-12-12 23:29   ` Andrii Nakryiko
2024-12-12 23:30 ` [PATCH bpf-next v3 0/2] " patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox