From patchwork Thu Jul 4 00:15:26 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrii Nakryiko X-Patchwork-Id: 13723084 X-Patchwork-Delegate: bpf@iogearbox.net Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B2A0E632 for ; Thu, 4 Jul 2024 00:15:38 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1720052138; cv=none; b=uK/7WGMUN3hE4fX51ByCLfevLwONovv6MERNWnzDmkBmCee4kBAIVjWtc1cd3QdJm4sb2NuhPzb2llEo6TKWXNl7LsGgBdwBp22M5xXcpIeoFq/01i8dnFGPxU9sV7i00rZwPJfChPWBx4xQcXg7+Pdm1nq2zjTU0ilFkULTDQ0= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1720052138; c=relaxed/simple; bh=rGJS1Gip8J3PXd2zZvyrPXYrmKtfnpq1nmbtNkpWzxE=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=dN+y4eLumVJA3KWTVna6GOp5gfMugHCszraQBu4I9wb5AwolHLIWvLghUeJX1XFcxts5Pvvh4CksgyEPCB6EnNrlEc/2Q1XkQ5uYavOSY1gfaMlioOxwOYVBrPEOKCYYK1tR3Qgj8rg7EKyJJFZNlIY8zhRTOjKKPh/J0Gp0QZo= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=fdoZdnCM; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="fdoZdnCM" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 2A5B8C2BD10; Thu, 4 Jul 2024 00:15:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1720052138; bh=rGJS1Gip8J3PXd2zZvyrPXYrmKtfnpq1nmbtNkpWzxE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=fdoZdnCMGSpcKGI3RvKbsZ2XlwOgULdiYXVDtQ0M9TF3PzvDHkcisz9UDDZSnf/cF NnB0WETzBQSJHWUq1hzzT3S1ovHJsltZ5+JPqmGm8iVgXD9J8SsTGvq+BQYwgLNCOY A6LKo742wbR2/ue3t3bd2uW/mpdx5fh5LgUeWswIQr725UGP5IGliiLX+nVvOr7Nbn ziJFtvNkJ4Y22txO4gyRvGJwQxkTQ9WomJ0/I/hhPivjsTGea88PHwsEGKC8Epry9X m4FzXUX/hVvGO6hKPTw+R3Y3LyVJUHjANpGNVm0LQ4SrwcSJdamBQPJ3Z0/9z07yv6 NC+m47LYr7p4Q== From: Andrii Nakryiko To: bpf@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net, martin.lau@kernel.org Cc: andrii@kernel.org, kernel-team@meta.com, Mykyta Yatsenko Subject: [PATCH bpf-next 1/2] bpftool: improve skeleton backwards compat with old buggy libbpfs Date: Wed, 3 Jul 2024 17:15:26 -0700 Message-ID: <20240704001527.754710-2-andrii@kernel.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240704001527.754710-1-andrii@kernel.org> References: <20240704001527.754710-1-andrii@kernel.org> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: bpf@iogearbox.net Old versions of libbpf don't handle varying sizes of bpf_map_skeleton struct correctly. As such, BPF skeleton generated by newest bpftool might not be compatible with older libbpf (though only when libbpf is used as a shared library), even though it, by design, should. Going forward libbpf will be fixed, plus we'll release bug fixed versions of relevant old libbpfs, but meanwhile try to mitigate from bpftool side by conservatively assuming older and smaller definition of bpf_map_skeleton, if possible. Meaning, if there are no struct_ops maps. If there are struct_ops, then presumably user would like to have auto-attaching logic and struct_ops map link placeholders, so use the full bpf_map_skeleton definition in that case. Co-developed-by: Mykyta Yatsenko Signed-off-by: Andrii Nakryiko Acked-by: Quentin Monnet --- tools/bpf/bpftool/gen.c | 46 ++++++++++++++++++++++++++++------------- 1 file changed, 32 insertions(+), 14 deletions(-) diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c index 51eaed76db97..70aaa32ddcc9 100644 --- a/tools/bpf/bpftool/gen.c +++ b/tools/bpf/bpftool/gen.c @@ -852,24 +852,41 @@ codegen_maps_skeleton(struct bpf_object *obj, size_t map_cnt, bool mmaped, bool { struct bpf_map *map; char ident[256]; - size_t i; + size_t i, map_sz; if (!map_cnt) return; + /* for backward compatibility with old libbpf versions that don't + * handle new BPF skeleton with new struct bpf_map_skeleton definition + * that includes link field, avoid specifying new increased size, + * unless we absolutely have to (i.e., if there are struct_ops maps + * present) + */ + map_sz = offsetof(struct bpf_map_skeleton, link); + if (populate_links) { + bpf_object__for_each_map(map, obj) { + if (bpf_map__type(map) == BPF_MAP_TYPE_STRUCT_OPS) { + map_sz = sizeof(struct bpf_map_skeleton); + break; + } + } + } + codegen("\ \n\ - \n\ + \n\ /* maps */ \n\ s->map_cnt = %zu; \n\ - s->map_skel_sz = sizeof(*s->maps); \n\ - s->maps = (struct bpf_map_skeleton *)calloc(s->map_cnt, s->map_skel_sz);\n\ + s->map_skel_sz = %zu; \n\ + s->maps = (struct bpf_map_skeleton *)calloc(s->map_cnt,\n\ + sizeof(*s->maps) > %zu ? sizeof(*s->maps) : %zu);\n\ if (!s->maps) { \n\ err = -ENOMEM; \n\ goto err; \n\ } \n\ ", - map_cnt + map_cnt, map_sz, map_sz, map_sz ); i = 0; bpf_object__for_each_map(map, obj) { @@ -878,23 +895,22 @@ codegen_maps_skeleton(struct bpf_object *obj, size_t map_cnt, bool mmaped, bool codegen("\ \n\ - \n\ - s->maps[%zu].name = \"%s\"; \n\ - s->maps[%zu].map = &obj->maps.%s; \n\ + \n\ + map = (struct bpf_map_skeleton *)((char *)s->maps + %zu * s->map_skel_sz);\n\ + map->name = \"%s\"; \n\ + map->map = &obj->maps.%s; \n\ ", - i, bpf_map__name(map), i, ident); + i, bpf_map__name(map), ident); /* memory-mapped internal maps */ if (mmaped && is_mmapable_map(map, ident, sizeof(ident))) { - printf("\ts->maps[%zu].mmaped = (void **)&obj->%s;\n", - i, ident); + printf("\tmap->mmaped = (void **)&obj->%s; \n", ident); } if (populate_links && bpf_map__type(map) == BPF_MAP_TYPE_STRUCT_OPS) { codegen("\ \n\ - s->maps[%zu].link = &obj->links.%s;\n\ - ", - i, ident); + map->link = &obj->links.%s; \n\ + ", ident); } i++; } @@ -1463,6 +1479,7 @@ static int do_skeleton(int argc, char **argv) %1$s__create_skeleton(struct %1$s *obj) \n\ { \n\ struct bpf_object_skeleton *s; \n\ + struct bpf_map_skeleton *map __attribute__((unused));\n\ int err; \n\ \n\ s = (struct bpf_object_skeleton *)calloc(1, sizeof(*s));\n\ @@ -1753,6 +1770,7 @@ static int do_subskeleton(int argc, char **argv) { \n\ struct %1$s *obj; \n\ struct bpf_object_subskeleton *s; \n\ + struct bpf_map_skeleton *map __attribute__((unused));\n\ int err; \n\ \n\ obj = (struct %1$s *)calloc(1, sizeof(*obj)); \n\ From patchwork Thu Jul 4 00:15:27 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrii Nakryiko X-Patchwork-Id: 13723085 X-Patchwork-Delegate: bpf@iogearbox.net Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 25B64632 for ; Thu, 4 Jul 2024 00:15:41 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1720052142; cv=none; b=GaDpEIHwV3D0nTSWDpdBA6ONbLKhoANwEQggHWeLWH69es0lbkOXWj1zkkd0Xvqjd5k+hNQCqYZFArd/fH0OgxXt+sY8T8mS25Q2rZELL3t+d+AbQyillSJuqKhi+C3v9xpUGITFYuAgTIFcJP0s8BllAZDTBCxHCMWMVAe85/A= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1720052142; c=relaxed/simple; bh=MnbqjXpJb9rIzkBCo8JD8b7O9EQw9Nt2C7Q8ywqgCHU=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=Qp9WTfP0eiRsF+mOC/dEvewIQDJaZxEzCtvStUMXjqagrCYUPu6fDRrFZ69MlQgChaKsr0xxuAbufq2b3ZTue2JhK2f94YByrkHxKFOz4XRnvCfvPZ/W/cv7yuBGOmH20BfZc8GUp05VsxCXBA45f5JybsDxlOUtOaJhUcxebLs= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=Y8qc5Imv; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="Y8qc5Imv" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 66341C2BD10; Thu, 4 Jul 2024 00:15:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1720052141; bh=MnbqjXpJb9rIzkBCo8JD8b7O9EQw9Nt2C7Q8ywqgCHU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Y8qc5ImvpFKrdLggHSeLZQxirsu04UNUBnz9gSWvoLybdJuwwp4PbmQyjJ6FQ2gcj Ov0rff6rAeFyk4hZuby//mFbJCSv440xenVmrGLj/Y7YufEtF9wvvF/PIK1/fOKqpW BesQTvTrxZXDD4EC5KUvXFOZPU3VCxta8Xzf1WO4pgzSg+Tkatru09THpvq5ukGoh/ PB8Bs69txMBLlUTSjP751CY6/mkg37n5vLh/ht8igN/MUgRZi90fEHZkQ1auNT3v1x 5xTului32k+IwB4zjNuIlSakL3MrwTdPDngg3/YIQxqUKPRd0xTuCkWyIH3yTtH+ML cU6r1sZBKRdyQ== From: Andrii Nakryiko To: bpf@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net, martin.lau@kernel.org Cc: andrii@kernel.org, kernel-team@meta.com, Mykyta Yatsenko Subject: [PATCH bpf-next 2/2] libbpf: fix BPF skeleton forward/backward compat handling Date: Wed, 3 Jul 2024 17:15:27 -0700 Message-ID: <20240704001527.754710-3-andrii@kernel.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240704001527.754710-1-andrii@kernel.org> References: <20240704001527.754710-1-andrii@kernel.org> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: bpf@iogearbox.net BPF skeleton was designed from day one to be extensible. Generated BPF skeleton code specifies actual sizes of map/prog/variable skeletons for that reason and libbpf is supposed to work with newer/older versions correctly. Unfortunately, it was missed that we implicitly embed hard-coded most up-to-date (according to libbpf's version of libbpf.h header used to compile BPF skeleton header) sizes of those strucs, which can differ from the actual sizes at runtime when libbpf is used as a shared library. We have a few places were we just index array of maps/progs/vars, which implicitly uses these potentially invalid sizes of structs. This patch aims to fix this problem going forward. Once this lands, we'll backport these changes in Github repo to create patched releases for older libbpfs. Fixes: d66562fba1ce ("libbpf: Add BPF object skeleton support") Fixes: 430025e5dca5 ("libbpf: Add subskeleton scaffolding") Co-developed-by: Mykyta Yatsenko Signed-off-by: Andrii Nakryiko Reviewed-by: Alan Maguire Acked-by: Eduard Zingerman --- tools/lib/bpf/libbpf.c | 47 ++++++++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index 4a28fac4908a..e92f933267d1 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -13712,14 +13712,15 @@ int libbpf_num_possible_cpus(void) static int populate_skeleton_maps(const struct bpf_object *obj, struct bpf_map_skeleton *maps, - size_t map_cnt) + size_t map_cnt, size_t map_skel_sz) { int i; for (i = 0; i < map_cnt; i++) { - struct bpf_map **map = maps[i].map; - const char *name = maps[i].name; - void **mmaped = maps[i].mmaped; + struct bpf_map_skeleton *map_skel = (void *)maps + i * map_skel_sz; + struct bpf_map **map = map_skel->map; + const char *name = map_skel->name; + void **mmaped = map_skel->mmaped; *map = bpf_object__find_map_by_name(obj, name); if (!*map) { @@ -13736,13 +13737,14 @@ static int populate_skeleton_maps(const struct bpf_object *obj, static int populate_skeleton_progs(const struct bpf_object *obj, struct bpf_prog_skeleton *progs, - size_t prog_cnt) + size_t prog_cnt, size_t prog_skel_sz) { int i; for (i = 0; i < prog_cnt; i++) { - struct bpf_program **prog = progs[i].prog; - const char *name = progs[i].name; + struct bpf_prog_skeleton *prog_skel = (void *)progs + i * prog_skel_sz; + struct bpf_program **prog = prog_skel->prog; + const char *name = prog_skel->name; *prog = bpf_object__find_program_by_name(obj, name); if (!*prog) { @@ -13783,13 +13785,13 @@ int bpf_object__open_skeleton(struct bpf_object_skeleton *s, } *s->obj = obj; - err = populate_skeleton_maps(obj, s->maps, s->map_cnt); + err = populate_skeleton_maps(obj, s->maps, s->map_cnt, s->map_skel_sz); if (err) { pr_warn("failed to populate skeleton maps for '%s': %d\n", s->name, err); return libbpf_err(err); } - err = populate_skeleton_progs(obj, s->progs, s->prog_cnt); + err = populate_skeleton_progs(obj, s->progs, s->prog_cnt, s->prog_skel_sz); if (err) { pr_warn("failed to populate skeleton progs for '%s': %d\n", s->name, err); return libbpf_err(err); @@ -13819,20 +13821,20 @@ int bpf_object__open_subskeleton(struct bpf_object_subskeleton *s) return libbpf_err(-errno); } - err = populate_skeleton_maps(s->obj, s->maps, s->map_cnt); + err = populate_skeleton_maps(s->obj, s->maps, s->map_cnt, s->map_skel_sz); if (err) { pr_warn("failed to populate subskeleton maps: %d\n", err); return libbpf_err(err); } - err = populate_skeleton_progs(s->obj, s->progs, s->prog_cnt); + err = populate_skeleton_progs(s->obj, s->progs, s->prog_cnt, s->prog_skel_sz); if (err) { pr_warn("failed to populate subskeleton maps: %d\n", err); return libbpf_err(err); } for (var_idx = 0; var_idx < s->var_cnt; var_idx++) { - var_skel = &s->vars[var_idx]; + var_skel = (void *)s->vars + var_idx * s->var_skel_sz; map = *var_skel->map; map_type_id = bpf_map__btf_value_type_id(map); map_type = btf__type_by_id(btf, map_type_id); @@ -13879,10 +13881,11 @@ int bpf_object__load_skeleton(struct bpf_object_skeleton *s) } for (i = 0; i < s->map_cnt; i++) { - struct bpf_map *map = *s->maps[i].map; + struct bpf_map_skeleton *map_skel = (void *)s->maps + i * s->map_skel_sz; + struct bpf_map *map = *map_skel->map; size_t mmap_sz = bpf_map_mmap_sz(map); int prot, map_fd = map->fd; - void **mmaped = s->maps[i].mmaped; + void **mmaped = map_skel->mmaped; if (!mmaped) continue; @@ -13930,8 +13933,9 @@ int bpf_object__attach_skeleton(struct bpf_object_skeleton *s) int i, err; for (i = 0; i < s->prog_cnt; i++) { - struct bpf_program *prog = *s->progs[i].prog; - struct bpf_link **link = s->progs[i].link; + struct bpf_prog_skeleton *prog_skel = (void *)s->progs + i * s->prog_skel_sz; + struct bpf_program *prog = *prog_skel->prog; + struct bpf_link **link = prog_skel->link; if (!prog->autoload || !prog->autoattach) continue; @@ -13970,8 +13974,9 @@ int bpf_object__attach_skeleton(struct bpf_object_skeleton *s) return 0; for (i = 0; i < s->map_cnt; i++) { - struct bpf_map *map = *s->maps[i].map; - struct bpf_link **link = s->maps[i].link; + struct bpf_map_skeleton *map_skel = (void *)s->maps + i * s->map_skel_sz; + struct bpf_map *map = *map_skel->map; + struct bpf_link **link = map_skel->link; if (!map->autocreate || !map->autoattach) continue; @@ -14000,7 +14005,8 @@ void bpf_object__detach_skeleton(struct bpf_object_skeleton *s) int i; for (i = 0; i < s->prog_cnt; i++) { - struct bpf_link **link = s->progs[i].link; + struct bpf_prog_skeleton *prog_skel = (void *)s->progs + i * s->prog_skel_sz; + struct bpf_link **link = prog_skel->link; bpf_link__destroy(*link); *link = NULL; @@ -14010,7 +14016,8 @@ void bpf_object__detach_skeleton(struct bpf_object_skeleton *s) return; for (i = 0; i < s->map_cnt; i++) { - struct bpf_link **link = s->maps[i].link; + struct bpf_map_skeleton *map_skel = (void *)s->maps + i * s->map_skel_sz; + struct bpf_link **link = map_skel->link; if (link) { bpf_link__destroy(*link);