From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4731FC6FD1D for ; Thu, 30 Mar 2023 22:05:36 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C51F76B0071; Thu, 30 Mar 2023 18:05:35 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C01CE6B0072; Thu, 30 Mar 2023 18:05:35 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AA2DF6B0075; Thu, 30 Mar 2023 18:05:35 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 956486B0071 for ; Thu, 30 Mar 2023 18:05:35 -0400 (EDT) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 5C1BF1A0823 for ; Thu, 30 Mar 2023 22:05:35 +0000 (UTC) X-FDA: 80626947030.07.573D88A Received: from mail-wm1-f49.google.com (mail-wm1-f49.google.com [209.85.128.49]) by imf09.hostedemail.com (Postfix) with ESMTP id 58FF514001B for ; Thu, 30 Mar 2023 22:05:32 +0000 (UTC) Authentication-Results: imf09.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=H1JOaltV; spf=pass (imf09.hostedemail.com: domain of olsajiri@gmail.com designates 209.85.128.49 as permitted sender) smtp.mailfrom=olsajiri@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1680213932; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=vn1y/TSQU/s4um/hYxW7MiRhdM9UUyXaGxedueCrmV0=; b=cSxIA4hKrdloPFvlzz2emhF/BRSIpnqMMDBreQmKz9YPSrrvK5BC3RntWVJwpQkfFqi2sY 3IcXbZB/AqtY+pnTJq2/nUo1F9NmTQDUhsbtjfVEtpD+3SMlOD20rQdlFQHXLSdwJwv3KS jQI/UHeDga2i8sHJm2S1Tgaa7KPhB98= ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=H1JOaltV; spf=pass (imf09.hostedemail.com: domain of olsajiri@gmail.com designates 209.85.128.49 as permitted sender) smtp.mailfrom=olsajiri@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1680213932; a=rsa-sha256; cv=none; b=3LdKCjW5iQhnrZigv6y0CzsnhHkXOePTUSNGLh9no0DEsOw2AKJZ1eBTJozI/Jtu+OAfS+ c9tJ9SSISCqa5WQVjU0qQhxx47r5fTQB5GPqs6M7z8+zkfHVJBASAEzdJNvc06ZZxvUkYf dKrSKFeziGNteKVYSz8f5nPVG3VeCyA= Received: by mail-wm1-f49.google.com with SMTP id o24-20020a05600c511800b003ef59905f26so12698254wms.2 for ; Thu, 30 Mar 2023 15:05:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1680213931; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:date:from:from:to :cc:subject:date:message-id:reply-to; bh=vn1y/TSQU/s4um/hYxW7MiRhdM9UUyXaGxedueCrmV0=; b=H1JOaltVeZZCCsf2+4gSMJWB/xFr9MQhe7OJsxlrKiZG9lfCq4J6Gq0qOb8EGSo6Kp PSmEEJ2X8+dihMWZMtPqtxZ3Yj/J9x0qpgZ5ra/zm9oeFwyDkEIa25sweaM1LsI4ejVz GD9TZGcmecknzIg8E23rJfVeb0Z0xCsIVEDULJ/UUJ8E1oTMr728LmE6kclGF6WJa4kp 5Nj6ON3y5s8eWVDJhEWTaJND31LCqECdsVXQLP5PHJlvHKcx7HP/RVv52jDk8G1Hvx24 yVXkVyERPeoA22ufzn/HUyrqIVRAcmSW3F4SZgBJu4hWUz/CmM2lQbikPj0VggOVGx0d g1JA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680213931; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:date:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=vn1y/TSQU/s4um/hYxW7MiRhdM9UUyXaGxedueCrmV0=; b=YBmglePBsT8qUdC5+cEVXzrzTYg31+X54Oi04I1nZaxOfMigPma7YuC8xh8HZl6ean 6kePxwHicOvRuMvukZOYHz29lCaauK41JegWD6AzGueH5V6T7WfLo2ILmmMe6M9lY6Ju /4018cNueyDcwDuHQPDrf4FYwdClPGMZgjGiNKjnEjVtYF8cLGD2IAOpniHCKkyYSX8F vv9fsvFeVpdXtRn6r5KUZ5t/cbUvhV7/OE+LIVO8yc4j4PNyhod+dteUwa7Sc18iB8w7 wfwstN6fy74Lttbhdlgcy1zwdIqDXE9hOtu4OWsWOIOtrD3OfUe+RksjQ62h88fG1CFx 3/cg== X-Gm-Message-State: AAQBX9fx3X6vzVKHHDHc90ocUVLwoucVRjlU1dryFchoc0FkaI1HnSZh xyr18g/UI8QEWbcpyYp8ZOA= X-Google-Smtp-Source: AKy350ZICrrUdVQb3X+xMlyj7UWwKz5tGv0mqHXM0pgeTHRgxsGX051MDWvlpYxSDr8+m0SD3v5WWA== X-Received: by 2002:a05:600c:21cd:b0:3ef:6e1c:3fe9 with SMTP id x13-20020a05600c21cd00b003ef6e1c3fe9mr12576272wmj.16.1680213930692; Thu, 30 Mar 2023 15:05:30 -0700 (PDT) Received: from krava ([83.240.63.154]) by smtp.gmail.com with ESMTPSA id s7-20020a5d4ec7000000b002c5544b3a69sm424300wrv.89.2023.03.30.15.05.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 30 Mar 2023 15:05:30 -0700 (PDT) From: Jiri Olsa X-Google-Original-From: Jiri Olsa Date: Fri, 31 Mar 2023 00:05:28 +0200 To: Andrii Nakryiko Cc: Alexei Starovoitov , Andrii Nakryiko , Hao Luo , Andrew Morton , Alexander Viro , Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Matthew Wilcox , bpf@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-perf-users@vger.kernel.org, Martin KaFai Lau , Song Liu , Yonghong Song , John Fastabend , KP Singh , Stanislav Fomichev , Daniel Borkmann , Namhyung Kim , Dave Chinner Subject: Re: [PATCHv3 bpf-next 5/9] selftests/bpf: Add read_buildid function Message-ID: References: <20230316170149.4106586-1-jolsa@kernel.org> <20230316170149.4106586-6-jolsa@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 58FF514001B X-Stat-Signature: e5ebpxbw9c8s16pdctzh61toy5mwz8iy X-Rspam-User: X-HE-Tag: 1680213932-111243 X-HE-Meta: U2FsdGVkX1/z2/BRwK1jh3NMWK/tMiJW7k5/K0ZFovTbCKemHlcRqY1Oc5Fa2DHKlgXOrlv8meTvJz1gbRweWwOYAIt4Yf/+gdSoY4tfPsMPhHEU+Rq9Ix9RJW+GQm/27xLqdfMlgHc6D+BXlnjKbOfWReG6rwrelpJrntj5lkFs5YB2L8DEirw5ThmigChcW06Mo7l3XeKRYSRNkg1ut4Nggb4lZTFHqwMnCQanzKVHaf5fXT2T0Aro0LpDNqSu2xZ7lSXW1LaL+JkY5QH/Z9Q4AhG/WL2NxJeWEAQR7k+8TOvSWUo2wVeMRnp5ukwNTYM7VPNyBL9gnyN2xuWwZSm9fz8Ff2MpgJvR0hUaN0RkO3rUoNhc60Syn07YB9QYJ6I6DJwQ2GLMHj9itRjNh1GX796bYy2ffbcO526HB72IkV+A8a22hIx+V1W9Uw2MSspCFDJyXmgpOaePbpiXBLvdtN6bt/pSCs+XaqJ1XS1KqLhO5p62PhaIpj3PO2/infaMQKgjiZ1TkdxrGD+e/WtXzDKY6+rst/enCuaMVqNrs03zVC63Ip1ZhmhbHykcCOD58E040jADAmlfoJi37lIsj07B4z6UdrGCIK1qooH938YePM1TyPaPEvnmQTUOxPNnd3Y6i5WISjkkBY0N+qYHIjNfDgaAHTjcYQQcrUxsjQdRUOmod6gJMG3nqyIICq/CuG0EaD6uv5p1Revpx3Td6fh4Q58Co6ClrLko5uRvBrWC8b+EpCC/gDL8nLBzaFsl8OoZqJQD7mjyztcZ06VY/gToBzf49HoiCjtxscXB806iecWWYCQo5T8ZsrEn4NIjHYE1bZ1wzb0MweEFWkfqMdtI8IxyZwkjoWSQOlvTqgD9gBqj5QczpWZN+pX8x72lqw90Yess+88SuzP5fzaosoZMQNKdhmekfgAqRH9IFNq5qnROJl7NjSlUylCYGOPM/Jdf0CHLYX76Xbo JWOZ+nMx gjDIDwBwC5b6PTaj/8tTcRD/zQj5tweieTXEzawcw1fHW0RAqIVqOrHE09fxg1d8oJAyp5aaYovZuIAgzr2nU3DEadLYo46EOsgXeme0ZOMwgG08NRi2Xk6dfsU4fzXmTvPCdbGs+4Pj+RUM8R4GdjAGqPy4cnFSX1IzZ8dCykXXOG0PPwtG/IAbVRsmto73/7EYsF13hcvZQc72EKfTT2nF7ogpWIMlKs5xKgQugezvLmPBk9a6aMBW+/BbvzBaHGkjXe5J6w25sQJUt2d1wgm5nI17EWzOeF5ElUmrzKBqMAsqMOOXwwAAZ163uc1zg+K8zyrba91fer/KOi68W6t2FxXC+Klzd+i+0Wb5eAGR4P8v8+C+abTNkTF9OIwfT1FjRTSNdCLW+df+PpssZpx0lt6ZgYec0FuaRK0o2XeaKIWA/34J5/AStKMDWT1Qy41FppZADhMOwCxEn6lbKWFxtE/b6e/2nWSVtBiRJm/dZXJLS0C/UNiVydmw7LQReFWRroM29G6g9SxRGOfv8vHmXnk/MMWlxBOiyowPTquPJ4wVhdHjCqeT/zaCK2CMrnbAjXXhjOayioyD3ClpbRJOXixuJJdgPdkXK8dLgC6pZ75JVxD/ANfrlW5NiCAZ/bAV1DPjJ1x8KdjX//nArbt6aDD4HAr10N4iswJkRUz1a6o9THJw1hplwHw== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Thu, Mar 16, 2023 at 03:23:03PM -0700, Andrii Nakryiko wrote: > On Thu, Mar 16, 2023 at 10:03 AM Jiri Olsa wrote: > > > > Adding read_build_id function that parses out build id from > > specified binary. > > > > It will replace extract_build_id and also be used in following > > changes. > > > > Signed-off-by: Jiri Olsa I'll send this separatelly as bpf/selftests fix so doesn't get lost > > --- > > tools/testing/selftests/bpf/trace_helpers.c | 86 +++++++++++++++++++++ > > tools/testing/selftests/bpf/trace_helpers.h | 5 ++ > > 2 files changed, 91 insertions(+) > > > > diff --git a/tools/testing/selftests/bpf/trace_helpers.c b/tools/testing/selftests/bpf/trace_helpers.c > > index 934bf28fc888..72b38a41f574 100644 > > --- a/tools/testing/selftests/bpf/trace_helpers.c > > +++ b/tools/testing/selftests/bpf/trace_helpers.c > > @@ -11,6 +11,9 @@ > > #include > > #include > > #include "trace_helpers.h" > > +#include > > +#include > > +#include > > > > #define TRACEFS_PIPE "/sys/kernel/tracing/trace_pipe" > > #define DEBUGFS_PIPE "/sys/kernel/debug/tracing/trace_pipe" > > @@ -234,3 +237,86 @@ ssize_t get_rel_offset(uintptr_t addr) > > fclose(f); > > return -EINVAL; > > } > > + > > +static int > > +parse_build_id_buf(const void *note_start, Elf32_Word note_size, > > + char *build_id) > > nit: single line ok > > should we pass buffer size instead of assuming at least BPF_BUILD_ID_SIZE below? ok > > > +{ > > + Elf32_Word note_offs = 0, new_offs; > > + > > + while (note_offs + sizeof(Elf32_Nhdr) < note_size) { > > + Elf32_Nhdr *nhdr = (Elf32_Nhdr *)(note_start + note_offs); > > + > > + if (nhdr->n_type == 3 && nhdr->n_namesz == sizeof("GNU") && > > + !strcmp((char *)(nhdr + 1), "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 (int) nhdr->n_descsz; > > + } > > + > > + new_offs = note_offs + sizeof(Elf32_Nhdr) + > > + ALIGN(nhdr->n_namesz, 4) + ALIGN(nhdr->n_descsz, 4); > > + if (new_offs >= note_size) > > + break; > > while condition() above would handle this, so this check appears not necessary? > > so just assign note_offs directly? good idea, it will simplify that > > > > + note_offs = new_offs; > > + } > > + > > + return -EINVAL; > > nit: -ENOENT or -ESRCH? I kept the same error as is in kernel, but ENOENT makes more sense > > > +} > > + > > +/* Reads binary from *path* file and returns it in the *build_id* > > + * which is expected to be at least BPF_BUILD_ID_SIZE bytes. > > + * Returns size of build id on success. On error the error value > > + * is returned. > > + */ > > +int read_build_id(const char *path, char *build_id) > > +{ > > + int fd, err = -EINVAL; > > + Elf *elf = NULL; > > + GElf_Ehdr ehdr; > > + size_t max, i; > > + > > + fd = open(path, O_RDONLY | O_CLOEXEC); > > + if (fd < 0) > > + return -errno; > > + > > + (void)elf_version(EV_CURRENT); > > + > > + elf = elf_begin(fd, ELF_C_READ, NULL); > > ELF_C_READ_MMAP ? ok > > > + if (!elf) > > + goto out; > > + if (elf_kind(elf) != ELF_K_ELF) > > + goto out; > > + if (gelf_getehdr(elf, &ehdr) == NULL) > > nit: !gelf_getehdr() ok > > > + goto out; > > + if (ehdr.e_ident[EI_CLASS] != ELFCLASS64) > > + goto out; > > does this have to be 64-bit specific?... you are using gelf stuff, you > can be bitness-agnostic here right, I don't think it's needed, will check > > > + > > + for (i = 0; i < ehdr.e_phnum; i++) { > > + GElf_Phdr mem, *phdr; > > + char *data; > > + > > + phdr = gelf_getphdr(elf, i, &mem); > > + if (!phdr) > > + goto out; > > + if (phdr->p_type != PT_NOTE) > > + continue; > > I don't know where ELF + build ID spec is (if at all), but it seems to > always be in the ".note.gnu.build-id" section, so should we check the > name here? this section name is not manadatory as stated in https://fedoraproject.org/wiki/RolandMcGrath/BuildID The new section is canonically called .note.gnu.build-id, but the name is not normative, and the section can be merged with other SHT_NOTE sections. The ELF note headers give name "GNU" and type 3 (NT_GNU_BUILD_ID) for a build ID note. > > > > + data = elf_rawfile(elf, &max); > > + if (!data) > > + goto out; > > + if (phdr->p_offset >= max || (phdr->p_offset + phdr->p_memsz >= max)) > > `phdr->p_offset + phdr->p_memsz == max` would be fine, no? right, will change thanks, jirka > > > + goto out; > > + err = parse_build_id_buf(data + phdr->p_offset, phdr->p_memsz, build_id); > > + if (err > 0) > > + goto out; > > + err = -EINVAL; > > + } > > + > > +out: > > + if (elf) > > + elf_end(elf); > > + close(fd); > > + return err; > > +} > > diff --git a/tools/testing/selftests/bpf/trace_helpers.h b/tools/testing/selftests/bpf/trace_helpers.h > > index 53efde0e2998..bc3b92057033 100644 > > --- a/tools/testing/selftests/bpf/trace_helpers.h > > +++ b/tools/testing/selftests/bpf/trace_helpers.h > > @@ -4,6 +4,9 @@ > > > > #include > > > > +#define __ALIGN_MASK(x, mask) (((x)+(mask))&~(mask)) > > +#define ALIGN(x, a) __ALIGN_MASK(x, (typeof(x))(a)-1) > > + > > struct ksym { > > long addr; > > char *name; > > @@ -23,4 +26,6 @@ void read_trace_pipe(void); > > ssize_t get_uprobe_offset(const void *addr); > > ssize_t get_rel_offset(uintptr_t addr); > > > > +int read_build_id(const char *path, char *build_id); > > + > > #endif > > -- > > 2.39.2 > >