netdev - Re: [PATCH bpf-next 1/3] bpftool: fix install for libbpf's internal header(s)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEf4BzbM1q-Ta8dO31E-zTszmGH3c+oAH6EeQtwv-TyQEXwp4g@mail.gmail.com>
Date:   Tue, 19 Oct 2021 16:44:15 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Quentin Monnet <quentin@...valent.com>
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>
Subject: Re: [PATCH bpf-next 1/3] bpftool: fix install for libbpf's internal header(s)

On Sat, Oct 9, 2021 at 2:03 PM Quentin Monnet <quentin@...valent.com> wrote:
>
> We recently updated bpftool's Makefile to make it install the headers
> from libbpf, instead of pulling them directly from libbpf's directory.
> There is also an additional header, internal to libbpf, that needs be
> installed. The way that bpftool's Makefile installs that particular
> header is currently correct, but would break if we were to modify
> $(LIBBPF_INTERNAL_HDRS) to make it point to more than one header.
>
> Use a static pattern rule instead, so that the Makefile can withstand
> the addition of other headers to install.
>
> The objective is simply to make the Makefile more robust. It should
> _not_ be read as an invitation to import more internal headers from
> libbpf into bpftool.
>
> Fixes: f012ade10b34 ("bpftool: Install libbpf headers instead of including the dir")
> Reported-by: Andrii Nakryiko <andrii@...nel.org>
> Signed-off-by: Quentin Monnet <quentin@...valent.com>
> ---
>  tools/bpf/bpftool/Makefile | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> index 9c2d13c513f0..2c510293f32b 100644
> --- a/tools/bpf/bpftool/Makefile
> +++ b/tools/bpf/bpftool/Makefile
> @@ -14,7 +14,7 @@ else
>    Q = @
>  endif
>
> -BPF_DIR = $(srctree)/tools/lib/bpf/
> +BPF_DIR = $(srctree)/tools/lib/bpf
>
>  ifneq ($(OUTPUT),)
>    _OUTPUT := $(OUTPUT)
> @@ -25,6 +25,7 @@ BOOTSTRAP_OUTPUT := $(_OUTPUT)/bootstrap/
>  LIBBPF_OUTPUT := $(_OUTPUT)/libbpf/
>  LIBBPF_DESTDIR := $(LIBBPF_OUTPUT)
>  LIBBPF_INCLUDE := $(LIBBPF_DESTDIR)/include
> +LIBBPF_HDRS_DIR := $(LIBBPF_INCLUDE)/bpf
>
>  LIBBPF = $(LIBBPF_OUTPUT)libbpf.a
>  LIBBPF_BOOTSTRAP_OUTPUT = $(BOOTSTRAP_OUTPUT)libbpf/
> @@ -32,7 +33,8 @@ LIBBPF_BOOTSTRAP = $(LIBBPF_BOOTSTRAP_OUTPUT)libbpf.a
>
>  # We need to copy nlattr.h which is not otherwise exported by libbpf, but still
>  # required by bpftool.
> -LIBBPF_INTERNAL_HDRS := nlattr.h
> +_LIBBPF_INTERNAL_HDRS := nlattr.h

Felt weird and ugly to have _LIBBPF_INTERNAL_HDRS, so I just inlined it.

> +LIBBPF_INTERNAL_HDRS := $(addprefix $(LIBBPF_HDRS_DIR)/,$(_LIBBPF_INTERNAL_HDRS))
>
>  ifeq ($(BPFTOOL_VERSION),)
>  BPFTOOL_VERSION := $(shell make -rR --no-print-directory -sC ../../.. kernelversion)
> @@ -45,10 +47,9 @@ $(LIBBPF): FORCE | $(LIBBPF_OUTPUT)
>         $(Q)$(MAKE) -C $(BPF_DIR) OUTPUT=$(LIBBPF_OUTPUT) \
>                 DESTDIR=$(LIBBPF_DESTDIR) prefix= $(LIBBPF) install_headers
>
> -$(LIBBPF_INCLUDE)/bpf/$(LIBBPF_INTERNAL_HDRS): \
> -               $(addprefix $(BPF_DIR),$(LIBBPF_INTERNAL_HDRS)) $(LIBBPF)
> -       $(call QUIET_INSTALL, bpf/$(notdir $@))
> -       $(Q)install -m 644 -t $(LIBBPF_INCLUDE)/bpf/ $(BPF_DIR)$(notdir $@)
> +$(LIBBPF_INTERNAL_HDRS): $(LIBBPF_HDRS_DIR)/%.h: $(BPF_DIR)/%.h $(LIBBPF)
> +       $(call QUIET_INSTALL, $@)
> +       $(Q)install -m 644 -t $(LIBBPF_HDRS_DIR) $<
>
>  $(LIBBPF_BOOTSTRAP): FORCE | $(LIBBPF_BOOTSTRAP_OUTPUT)
>         $(Q)$(MAKE) -C $(BPF_DIR) OUTPUT=$(LIBBPF_BOOTSTRAP_OUTPUT) \
> @@ -150,7 +151,7 @@ BOOTSTRAP_OBJS = $(addprefix $(BOOTSTRAP_OUTPUT),main.o common.o json_writer.o g
>  $(BOOTSTRAP_OBJS): $(LIBBPF_BOOTSTRAP)
>
>  OBJS = $(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o
> -$(OBJS): $(LIBBPF) $(LIBBPF_INCLUDE)/bpf/$(LIBBPF_INTERNAL_HDRS)
> +$(OBJS): $(LIBBPF) $(LIBBPF_INTERNAL_HDRS)
>
>  VMLINUX_BTF_PATHS ?= $(if $(O),$(O)/vmlinux)                           \
>                      $(if $(KBUILD_OUTPUT),$(KBUILD_OUTPUT)/vmlinux)    \
> --
> 2.30.2
>