| From 53e67d8d95dcfb27706140da5ecbefbfcd97c989 Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Tue, 25 Oct 2022 16:03:23 +0100 |
| Subject: bpftool: Remove asserts from JIT disassembler |
| MIME-Version: 1.0 |
| Content-Type: text/plain; charset=UTF-8 |
| Content-Transfer-Encoding: 8bit |
| |
| From: Quentin Monnet <quentin@isovalent.com> |
| |
| [ Upstream commit 55b4de58d0e2aca810ed2b198a0173640300acf8 ] |
| |
| The JIT disassembler in bpftool is the only components (with the JSON |
| writer) using asserts to check the return values of functions. But it |
| does not do so in a consistent way, and diasm_print_insn() returns no |
| value, although sometimes the operation failed. |
| |
| Remove the asserts, and instead check the return values, print messages |
| on errors, and propagate the error to the caller from prog.c. |
| |
| Remove the inclusion of assert.h from jit_disasm.c, and also from map.c |
| where it is unused. |
| |
| Signed-off-by: Quentin Monnet <quentin@isovalent.com> |
| Tested-by: Niklas Sรถderlund <niklas.soderlund@corigine.com> |
| Acked-by: Song Liu <song@kernel.org> |
| Link: https://lore.kernel.org/r/20221025150329.97371-3-quentin@isovalent.com |
| Signed-off-by: Alexei Starovoitov <ast@kernel.org> |
| Stable-dep-of: ef3ba8c258ee ("bpftool: fix potential NULL pointer dereferencing in prog_dump()") |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| tools/bpf/bpftool/jit_disasm.c | 51 +++++++++++++++++++++++----------- |
| tools/bpf/bpftool/main.h | 25 +++++++++-------- |
| tools/bpf/bpftool/map.c | 1 - |
| tools/bpf/bpftool/prog.c | 15 ++++++---- |
| 4 files changed, 57 insertions(+), 35 deletions(-) |
| |
| diff --git a/tools/bpf/bpftool/jit_disasm.c b/tools/bpf/bpftool/jit_disasm.c |
| index aaf99a0168c90..fe23c9669a876 100644 |
| --- a/tools/bpf/bpftool/jit_disasm.c |
| +++ b/tools/bpf/bpftool/jit_disasm.c |
| @@ -16,7 +16,6 @@ |
| #include <stdarg.h> |
| #include <stdint.h> |
| #include <stdlib.h> |
| -#include <assert.h> |
| #include <unistd.h> |
| #include <string.h> |
| #include <bfd.h> |
| @@ -29,14 +28,18 @@ |
| #include "json_writer.h" |
| #include "main.h" |
| |
| -static void get_exec_path(char *tpath, size_t size) |
| +static int get_exec_path(char *tpath, size_t size) |
| { |
| const char *path = "/proc/self/exe"; |
| ssize_t len; |
| |
| len = readlink(path, tpath, size - 1); |
| - assert(len > 0); |
| + if (len <= 0) |
| + return -1; |
| + |
| tpath[len] = 0; |
| + |
| + return 0; |
| } |
| |
| static int oper_count; |
| @@ -97,30 +100,39 @@ static int fprintf_json_styled(void *out, |
| return r; |
| } |
| |
| -void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes, |
| - const char *arch, const char *disassembler_options, |
| - const struct btf *btf, |
| - const struct bpf_prog_linfo *prog_linfo, |
| - __u64 func_ksym, unsigned int func_idx, |
| - bool linum) |
| +int disasm_print_insn(unsigned char *image, ssize_t len, int opcodes, |
| + const char *arch, const char *disassembler_options, |
| + const struct btf *btf, |
| + const struct bpf_prog_linfo *prog_linfo, |
| + __u64 func_ksym, unsigned int func_idx, |
| + bool linum) |
| { |
| const struct bpf_line_info *linfo = NULL; |
| disassembler_ftype disassemble; |
| + int count, i, pc = 0, err = -1; |
| struct disassemble_info info; |
| unsigned int nr_skip = 0; |
| - int count, i, pc = 0; |
| char tpath[PATH_MAX]; |
| bfd *bfdf; |
| |
| if (!len) |
| - return; |
| + return -1; |
| |
| memset(tpath, 0, sizeof(tpath)); |
| - get_exec_path(tpath, sizeof(tpath)); |
| + if (get_exec_path(tpath, sizeof(tpath))) { |
| + p_err("failed to create disasembler (get_exec_path)"); |
| + return -1; |
| + } |
| |
| bfdf = bfd_openr(tpath, NULL); |
| - assert(bfdf); |
| - assert(bfd_check_format(bfdf, bfd_object)); |
| + if (!bfdf) { |
| + p_err("failed to create disassembler (bfd_openr)"); |
| + return -1; |
| + } |
| + if (!bfd_check_format(bfdf, bfd_object)) { |
| + p_err("failed to create disassembler (bfd_check_format)"); |
| + goto exit_close; |
| + } |
| |
| if (json_output) |
| init_disassemble_info_compat(&info, stdout, |
| @@ -139,7 +151,7 @@ void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes, |
| bfdf->arch_info = inf; |
| } else { |
| p_err("No libbfd support for %s", arch); |
| - return; |
| + goto exit_close; |
| } |
| } |
| |
| @@ -160,7 +172,10 @@ void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes, |
| #else |
| disassemble = disassembler(bfdf); |
| #endif |
| - assert(disassemble); |
| + if (!disassemble) { |
| + p_err("failed to create disassembler"); |
| + goto exit_close; |
| + } |
| |
| if (json_output) |
| jsonw_start_array(json_wtr); |
| @@ -224,7 +239,11 @@ void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes, |
| if (json_output) |
| jsonw_end_array(json_wtr); |
| |
| + err = 0; |
| + |
| +exit_close: |
| bfd_close(bfdf); |
| + return err; |
| } |
| |
| int disasm_init(void) |
| diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h |
| index 90caa42aac4cf..834f8317b855f 100644 |
| --- a/tools/bpf/bpftool/main.h |
| +++ b/tools/bpf/bpftool/main.h |
| @@ -193,22 +193,23 @@ int map_parse_fd_and_info(int *argc, char ***argv, void *info, __u32 *info_len); |
| |
| struct bpf_prog_linfo; |
| #ifdef HAVE_LIBBFD_SUPPORT |
| -void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes, |
| - const char *arch, const char *disassembler_options, |
| - const struct btf *btf, |
| - const struct bpf_prog_linfo *prog_linfo, |
| - __u64 func_ksym, unsigned int func_idx, |
| - bool linum); |
| +int disasm_print_insn(unsigned char *image, ssize_t len, int opcodes, |
| + const char *arch, const char *disassembler_options, |
| + const struct btf *btf, |
| + const struct bpf_prog_linfo *prog_linfo, |
| + __u64 func_ksym, unsigned int func_idx, |
| + bool linum); |
| int disasm_init(void); |
| #else |
| static inline |
| -void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes, |
| - const char *arch, const char *disassembler_options, |
| - const struct btf *btf, |
| - const struct bpf_prog_linfo *prog_linfo, |
| - __u64 func_ksym, unsigned int func_idx, |
| - bool linum) |
| +int disasm_print_insn(unsigned char *image, ssize_t len, int opcodes, |
| + const char *arch, const char *disassembler_options, |
| + const struct btf *btf, |
| + const struct bpf_prog_linfo *prog_linfo, |
| + __u64 func_ksym, unsigned int func_idx, |
| + bool linum) |
| { |
| + return 0; |
| } |
| static inline int disasm_init(void) |
| { |
| diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c |
| index 72ef9ddae2609..c775a677688ae 100644 |
| --- a/tools/bpf/bpftool/map.c |
| +++ b/tools/bpf/bpftool/map.c |
| @@ -1,7 +1,6 @@ |
| // SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) |
| /* Copyright (C) 2017-2018 Netronome Systems, Inc. */ |
| |
| -#include <assert.h> |
| #include <errno.h> |
| #include <fcntl.h> |
| #include <linux/err.h> |
| diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c |
| index afb28fcc66786..75f71c3ad4f62 100644 |
| --- a/tools/bpf/bpftool/prog.c |
| +++ b/tools/bpf/bpftool/prog.c |
| @@ -744,10 +744,11 @@ prog_dump(struct bpf_prog_info *info, enum dump_mode mode, |
| printf("%s:\n", sym_name); |
| } |
| |
| - disasm_print_insn(img, lens[i], opcodes, |
| - name, disasm_opt, btf, |
| - prog_linfo, ksyms[i], i, |
| - linum); |
| + if (disasm_print_insn(img, lens[i], opcodes, |
| + name, disasm_opt, btf, |
| + prog_linfo, ksyms[i], i, |
| + linum)) |
| + goto exit_free; |
| |
| img += lens[i]; |
| |
| @@ -760,8 +761,10 @@ prog_dump(struct bpf_prog_info *info, enum dump_mode mode, |
| if (json_output) |
| jsonw_end_array(json_wtr); |
| } else { |
| - disasm_print_insn(buf, member_len, opcodes, name, |
| - disasm_opt, btf, NULL, 0, 0, false); |
| + if (disasm_print_insn(buf, member_len, opcodes, name, |
| + disasm_opt, btf, NULL, 0, 0, |
| + false)) |
| + goto exit_free; |
| } |
| } else if (visual) { |
| if (json_output) |
| -- |
| 2.43.0 |
| |