On Wed, Dec 11, 2024 at 8:40 AM Alastair Robertson <ajor@xxxxxxxx> 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@xxxxxxxx>
> ---
> 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
>
