Re: [PATCH 01/15] bpf: Move stack_map_get_build_id into lib

From: Arnaldo Carvalho de Melo
Date: Tue Dec 15 2020 - 10:50:29 EST

Next message: Arnd Bergmann: "Re: [PATCH v2 1/2] arm64: dts: broadcom: clear the warnings caused by empty dma-ranges"
Previous message: Ricardo Ribalda: "[PATCH v4 9/9] media: uvcvideo: Implement UVC_QUIRK_PRIVACY_DURING_STREAM"
In reply to: Jiri Olsa: "[PATCH 01/15] bpf: Move stack_map_get_build_id into lib"
Next in thread: Jiri Olsa: "[PATCH 02/15] bpf: Add size arg to build_id_parse function"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Em Mon, Dec 14, 2020 at 11:54:43AM +0100, Jiri Olsa escreveu:
> Moving stack_map_get_build_id into lib with
> declaration in linux/buildid.h header:
>
> int build_id_parse(struct vm_area_struct *vma, unsigned char *build_id);
>
> This function returns build id for given struct vm_area_struct.
> There is no functional change to stack_map_get_build_id function.

Alexei, if you're ok with this, can you please process it? Linus will
find strange if I send kernel bits, as we agreed that my tools pull
requests would be just for tooling.

- Arnaldo

> Cc: Alexei Starovoitov <ast@xxxxxxxxxx>
> Acked-by: Song Liu <songliubraving@xxxxxx>
> Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> ---
> include/linux/buildid.h | 11 ++++
> kernel/bpf/stackmap.c | 143 ++--------------------------------------
> lib/Makefile | 3 +-
> lib/buildid.c | 136 ++++++++++++++++++++++++++++++++++++++
> 4 files changed, 153 insertions(+), 140 deletions(-)
> create mode 100644 include/linux/buildid.h
> create mode 100644 lib/buildid.c
>
> diff --git a/include/linux/buildid.h b/include/linux/buildid.h
> new file mode 100644
> index 000000000000..08028a212589
> --- /dev/null
> +++ b/include/linux/buildid.h
> @@ -0,0 +1,11 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_BUILDID_H
> +#define _LINUX_BUILDID_H
> +
> +#include <linux/mm_types.h>
> +
> +#define BUILD_ID_SIZE_MAX 20
> +
> +int build_id_parse(struct vm_area_struct *vma, unsigned char *build_id);
> +
> +#endif
> diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
> index 06065fa27124..d21512fbfa9a 100644
> --- a/kernel/bpf/stackmap.c
> +++ b/kernel/bpf/stackmap.c
> @@ -7,10 +7,9 @@
> #include <linux/kernel.h>
> #include <linux/stacktrace.h>
> #include <linux/perf_event.h>
> -#include <linux/elf.h>
> -#include <linux/pagemap.h>
> #include <linux/irq_work.h>
> #include <linux/btf_ids.h>
> +#include <linux/buildid.h>
> #include "percpu_freelist.h"
>
> #define STACK_CREATE_FLAG_MASK \
> @@ -153,140 +152,6 @@ static struct bpf_map *stack_map_alloc(union bpf_attr *attr)
> return ERR_PTR(err);
> }
>
> -#define BPF_BUILD_ID 3
> -/*
> - * Parse build id from the note segment. This logic can be shared between
> - * 32-bit and 64-bit system, because Elf32_Nhdr and Elf64_Nhdr are
> - * identical.
> - */
> -static inline int stack_map_parse_build_id(void *page_addr,
> - unsigned char *build_id,
> - void *note_start,
> - Elf32_Word note_size)
> -{
> - Elf32_Word note_offs = 0, new_offs;
> -
> - /* check for overflow */
> - if (note_start < page_addr || note_start + note_size < note_start)
> - return -EINVAL;
> -
> - /* only supports note that fits in the first page */
> - if (note_start + note_size > page_addr + PAGE_SIZE)
> - return -EINVAL;
> -
> - while (note_offs + sizeof(Elf32_Nhdr) < note_size) {
> - Elf32_Nhdr *nhdr = (Elf32_Nhdr *)(note_start + note_offs);
> -
> - if (nhdr->n_type == BPF_BUILD_ID &&
> - nhdr->n_namesz == sizeof("GNU") &&
> - nhdr->n_descsz > 0 &&
> - nhdr->n_descsz <= BPF_BUILD_ID_SIZE) {
> - memcpy(build_id,
> - note_start + note_offs +
> - ALIGN(sizeof("GNU"), 4) + sizeof(Elf32_Nhdr),
> - nhdr->n_descsz);
> - memset(build_id + nhdr->n_descsz, 0,
> - BPF_BUILD_ID_SIZE - nhdr->n_descsz);
> - return 0;
> - }
> - new_offs = note_offs + sizeof(Elf32_Nhdr) +
> - ALIGN(nhdr->n_namesz, 4) + ALIGN(nhdr->n_descsz, 4);
> - if (new_offs <= note_offs) /* overflow */
> - break;
> - note_offs = new_offs;
> - }
> - return -EINVAL;
> -}
> -
> -/* Parse build ID from 32-bit ELF */
> -static int stack_map_get_build_id_32(void *page_addr,
> - unsigned char *build_id)
> -{
> - Elf32_Ehdr *ehdr = (Elf32_Ehdr *)page_addr;
> - Elf32_Phdr *phdr;
> - int i;
> -
> - /* only supports phdr that fits in one page */
> - if (ehdr->e_phnum >
> - (PAGE_SIZE - sizeof(Elf32_Ehdr)) / sizeof(Elf32_Phdr))
> - return -EINVAL;
> -
> - phdr = (Elf32_Phdr *)(page_addr + sizeof(Elf32_Ehdr));
> -
> - for (i = 0; i < ehdr->e_phnum; ++i) {
> - if (phdr[i].p_type == PT_NOTE &&
> - !stack_map_parse_build_id(page_addr, build_id,
> - page_addr + phdr[i].p_offset,
> - phdr[i].p_filesz))
> - return 0;
> - }
> - return -EINVAL;
> -}
> -
> -/* Parse build ID from 64-bit ELF */
> -static int stack_map_get_build_id_64(void *page_addr,
> - unsigned char *build_id)
> -{
> - Elf64_Ehdr *ehdr = (Elf64_Ehdr *)page_addr;
> - Elf64_Phdr *phdr;
> - int i;
> -
> - /* only supports phdr that fits in one page */
> - if (ehdr->e_phnum >
> - (PAGE_SIZE - sizeof(Elf64_Ehdr)) / sizeof(Elf64_Phdr))
> - return -EINVAL;
> -
> - phdr = (Elf64_Phdr *)(page_addr + sizeof(Elf64_Ehdr));
> -
> - for (i = 0; i < ehdr->e_phnum; ++i) {
> - if (phdr[i].p_type == PT_NOTE &&
> - !stack_map_parse_build_id(page_addr, build_id,
> - page_addr + phdr[i].p_offset,
> - phdr[i].p_filesz))
> - return 0;
> - }
> - return -EINVAL;
> -}
> -
> -/* Parse build ID of ELF file mapped to vma */
> -static int stack_map_get_build_id(struct vm_area_struct *vma,
> - unsigned char *build_id)
> -{
> - Elf32_Ehdr *ehdr;
> - struct page *page;
> - void *page_addr;
> - int ret;
> -
> - /* only works for page backed storage */
> - if (!vma->vm_file)
> - return -EINVAL;
> -
> - page = find_get_page(vma->vm_file->f_mapping, 0);
> - if (!page)
> - return -EFAULT; /* page not mapped */
> -
> - ret = -EINVAL;
> - page_addr = kmap_atomic(page);
> - ehdr = (Elf32_Ehdr *)page_addr;
> -
> - /* compare magic x7f "ELF" */
> - if (memcmp(ehdr->e_ident, ELFMAG, SELFMAG) != 0)
> - goto out;
> -
> - /* only support executable file and shared object file */
> - if (ehdr->e_type != ET_EXEC && ehdr->e_type != ET_DYN)
> - goto out;
> -
> - if (ehdr->e_ident[EI_CLASS] == ELFCLASS32)
> - ret = stack_map_get_build_id_32(page_addr, build_id);
> - else if (ehdr->e_ident[EI_CLASS] == ELFCLASS64)
> - ret = stack_map_get_build_id_64(page_addr, build_id);
> -out:
> - kunmap_atomic(page_addr);
> - put_page(page);
> - return ret;
> -}
> -
> static void stack_map_get_build_id_offset(struct bpf_stack_build_id *id_offs,
> u64 *ips, u32 trace_nr, bool user)
> {
> @@ -327,18 +192,18 @@ static void stack_map_get_build_id_offset(struct bpf_stack_build_id *id_offs,
> for (i = 0; i < trace_nr; i++) {
> id_offs[i].status = BPF_STACK_BUILD_ID_IP;
> id_offs[i].ip = ips[i];
> - memset(id_offs[i].build_id, 0, BPF_BUILD_ID_SIZE);
> + memset(id_offs[i].build_id, 0, BUILD_ID_SIZE_MAX);
> }
> return;
> }
>
> for (i = 0; i < trace_nr; i++) {
> vma = find_vma(current->mm, ips[i]);
> - if (!vma || stack_map_get_build_id(vma, id_offs[i].build_id)) {
> + if (!vma || build_id_parse(vma, id_offs[i].build_id)) {
> /* per entry fall back to ips */
> id_offs[i].status = BPF_STACK_BUILD_ID_IP;
> id_offs[i].ip = ips[i];
> - memset(id_offs[i].build_id, 0, BPF_BUILD_ID_SIZE);
> + memset(id_offs[i].build_id, 0, BUILD_ID_SIZE_MAX);
> continue;
> }
> id_offs[i].offset = (vma->vm_pgoff << PAGE_SHIFT) + ips[i]
> diff --git a/lib/Makefile b/lib/Makefile
> index ce45af50983a..f4858f5e9215 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -36,7 +36,8 @@ lib-y := ctype.o string.o vsprintf.o cmdline.o \
> flex_proportions.o ratelimit.o show_mem.o \
> is_single_threaded.o plist.o decompress.o kobject_uevent.o \
> earlycpio.o seq_buf.o siphash.o dec_and_lock.o \
> - nmi_backtrace.o nodemask.o win_minmax.o memcat_p.o
> + nmi_backtrace.o nodemask.o win_minmax.o memcat_p.o \
> + buildid.o
>
> lib-$(CONFIG_PRINTK) += dump_stack.o
> lib-$(CONFIG_SMP) += cpumask.o
> diff --git a/lib/buildid.c b/lib/buildid.c
> new file mode 100644
> index 000000000000..4a4f520c0e29
> --- /dev/null
> +++ b/lib/buildid.c
> @@ -0,0 +1,136 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/buildid.h>
> +#include <linux/elf.h>
> +#include <linux/pagemap.h>
> +
> +#define BUILD_ID 3
> +/*
> + * Parse build id from the note segment. This logic can be shared between
> + * 32-bit and 64-bit system, because Elf32_Nhdr and Elf64_Nhdr are
> + * identical.
> + */
> +static inline int parse_build_id(void *page_addr,
> + unsigned char *build_id,
> + void *note_start,
> + Elf32_Word note_size)
> +{
> + Elf32_Word note_offs = 0, new_offs;
> +
> + /* check for overflow */
> + if (note_start < page_addr || note_start + note_size < note_start)
> + return -EINVAL;
> +
> + /* only supports note that fits in the first page */
> + if (note_start + note_size > page_addr + PAGE_SIZE)
> + return -EINVAL;
> +
> + while (note_offs + sizeof(Elf32_Nhdr) < note_size) {
> + Elf32_Nhdr *nhdr = (Elf32_Nhdr *)(note_start + note_offs);
> +
> + if (nhdr->n_type == BUILD_ID &&
> + nhdr->n_namesz == sizeof("GNU") &&
> + nhdr->n_descsz > 0 &&
> + nhdr->n_descsz <= BUILD_ID_SIZE_MAX) {
> + memcpy(build_id,
> + note_start + note_offs +
> + ALIGN(sizeof("GNU"), 4) + sizeof(Elf32_Nhdr),
> + nhdr->n_descsz);
> + memset(build_id + nhdr->n_descsz, 0,
> + BUILD_ID_SIZE_MAX - nhdr->n_descsz);
> + return 0;
> + }
> + new_offs = note_offs + sizeof(Elf32_Nhdr) +
> + ALIGN(nhdr->n_namesz, 4) + ALIGN(nhdr->n_descsz, 4);
> + if (new_offs <= note_offs) /* overflow */
> + break;
> + note_offs = new_offs;
> + }
> + return -EINVAL;
> +}
> +
> +/* Parse build ID from 32-bit ELF */
> +static int get_build_id_32(void *page_addr, unsigned char *build_id)
> +{
> + Elf32_Ehdr *ehdr = (Elf32_Ehdr *)page_addr;
> + Elf32_Phdr *phdr;
> + int i;
> +
> + /* only supports phdr that fits in one page */
> + if (ehdr->e_phnum >
> + (PAGE_SIZE - sizeof(Elf32_Ehdr)) / sizeof(Elf32_Phdr))
> + return -EINVAL;
> +
> + phdr = (Elf32_Phdr *)(page_addr + sizeof(Elf32_Ehdr));
> +
> + for (i = 0; i < ehdr->e_phnum; ++i) {
> + if (phdr[i].p_type == PT_NOTE &&
> + !parse_build_id(page_addr, build_id,
> + page_addr + phdr[i].p_offset,
> + phdr[i].p_filesz))
> + return 0;
> + }
> + return -EINVAL;
> +}
> +
> +/* Parse build ID from 64-bit ELF */
> +static int get_build_id_64(void *page_addr, unsigned char *build_id)
> +{
> + Elf64_Ehdr *ehdr = (Elf64_Ehdr *)page_addr;
> + Elf64_Phdr *phdr;
> + int i;
> +
> + /* only supports phdr that fits in one page */
> + if (ehdr->e_phnum >
> + (PAGE_SIZE - sizeof(Elf64_Ehdr)) / sizeof(Elf64_Phdr))
> + return -EINVAL;
> +
> + phdr = (Elf64_Phdr *)(page_addr + sizeof(Elf64_Ehdr));
> +
> + for (i = 0; i < ehdr->e_phnum; ++i) {
> + if (phdr[i].p_type == PT_NOTE &&
> + !parse_build_id(page_addr, build_id,
> + page_addr + phdr[i].p_offset,
> + phdr[i].p_filesz))
> + return 0;
> + }
> + return -EINVAL;
> +}
> +
> +/* Parse build ID of ELF file mapped to vma */
> +int build_id_parse(struct vm_area_struct *vma, unsigned char *build_id)
> +{
> + Elf32_Ehdr *ehdr;
> + struct page *page;
> + void *page_addr;
> + int ret;
> +
> + /* only works for page backed storage */
> + if (!vma->vm_file)
> + return -EINVAL;
> +
> + page = find_get_page(vma->vm_file->f_mapping, 0);
> + if (!page)
> + return -EFAULT; /* page not mapped */
> +
> + ret = -EINVAL;
> + page_addr = kmap_atomic(page);
> + ehdr = (Elf32_Ehdr *)page_addr;
> +
> + /* compare magic x7f "ELF" */
> + if (memcmp(ehdr->e_ident, ELFMAG, SELFMAG) != 0)
> + goto out;
> +
> + /* only support executable file and shared object file */
> + if (ehdr->e_type != ET_EXEC && ehdr->e_type != ET_DYN)
> + goto out;
> +
> + if (ehdr->e_ident[EI_CLASS] == ELFCLASS32)
> + ret = get_build_id_32(page_addr, build_id);
> + else if (ehdr->e_ident[EI_CLASS] == ELFCLASS64)
> + ret = get_build_id_64(page_addr, build_id);
> +out:
> + kunmap_atomic(page_addr);
> + put_page(page);
> + return ret;
> +}
> --
> 2.26.2
>

--

- Arnaldo

Next message: Arnd Bergmann: "Re: [PATCH v2 1/2] arm64: dts: broadcom: clear the warnings caused by empty dma-ranges"
Previous message: Ricardo Ribalda: "[PATCH v4 9/9] media: uvcvideo: Implement UVC_QUIRK_PRIVACY_DURING_STREAM"
In reply to: Jiri Olsa: "[PATCH 01/15] bpf: Move stack_map_get_build_id into lib"
Next in thread: Jiri Olsa: "[PATCH 02/15] bpf: Add size arg to build_id_parse function"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]