Messages in this thread Patch in this message | | | Subject | Re: [PATCH bpf-next] libbpf: Optimized return value in libbpf_strerror when errno is libbpf errno | From | Daniel Borkmann <> | Date | Wed, 7 Dec 2022 10:09:07 +0100 |
| |
On 12/7/22 1:00 AM, Andrii Nakryiko wrote: > On Mon, Dec 5, 2022 at 1:11 PM Daniel Borkmann <daniel@iogearbox.net> wrote: >> >> On 12/3/22 10:37 AM, Xin Liu wrote: >>> This is a small improvement in libbpf_strerror. When libbpf_strerror >>> is used to obtain the system error description, if the length of the >>> buf is insufficient, libbpf_sterror returns ERANGE and sets errno to >>> ERANGE. >>> >>> However, this processing is not performed when the error code >>> customized by libbpf is obtained. Make some minor improvements here, >>> return -ERANGE and set errno to ERANGE when buf is not enough for >>> custom description. >>> >>> Signed-off-by: Xin Liu <liuxin350@huawei.com> >>> --- >>> tools/lib/bpf/libbpf_errno.c | 6 ++++++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/tools/lib/bpf/libbpf_errno.c b/tools/lib/bpf/libbpf_errno.c >>> index 96f67a772a1b..48ce7d5b5bf9 100644 >>> --- a/tools/lib/bpf/libbpf_errno.c >>> +++ b/tools/lib/bpf/libbpf_errno.c >>> @@ -54,10 +54,16 @@ int libbpf_strerror(int err, char *buf, size_t size) >>> >>> if (err < __LIBBPF_ERRNO__END) { >>> const char *msg; >>> + size_t msg_size; >>> >>> msg = libbpf_strerror_table[ERRNO_OFFSET(err)]; >>> snprintf(buf, size, "%s", msg); >>> buf[size - 1] = '\0'; >>> + >>> + msg_size = strlen(msg); >>> + if (msg_size >= size) >>> + return libbpf_err(-ERANGE); >> >> Given this is related to libbpf_strerror_table[] where the error strings are known >> lets do compile-time error instead. All callers should pass in a buffer of STRERR_BUFSIZE >> size in libbpf. > > That sounds a bit too pessimistic?.. If the actual error message fits > in the buffer, why return -ERANGE just because theoretically some > error descriptions might fit? > > But I don't think we need to calculate strlen(). snprintf above > returns the number of bytes required to print a full string, even if > it was truncated. So just comparing snprintf's result to size should > be enough.
I meant sth like below. For example if we were to shrink STRERR_BUFSIZE down to 32 for testing, you'd then get:
# make libbpf_errno.o gcc -g -O2 -std=gnu89 -Wbad-function-cast -Wdeclaration-after-statement -Wformat-security -Wformat-y2k -Winit-self -Wmissing-declarations -Wmissing-prototypes -Wnested-externs -Wno-system-headers -Wold-style-definition -Wpacked -Wredundant-decls -Wstrict-prototypes -Wswitch-default -Wswitch-enum -Wundef -Wwrite-strings -Wformat -Wno-type-limits -Wstrict-aliasing=3 -Wshadow -Wno-switch-enum -Werror -Wall -I. -I/home/darkstar/trees/bpf-next/tools/include -I/home/darkstar/trees/bpf-next/tools/include/uapi -fvisibility=hidden -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -c -o libbpf_errno.o libbpf_errno.c libbpf_errno.c:27:31: error: initializer-string for array of chars is too long [-Werror] 27 | [ERRCODE_OFFSET(KVERSION)] = "'version' section incorrect or lost", | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ libbpf_errno.c:27:31: note: (near initialization for ‘libbpf_strerror_table[2]’) libbpf_errno.c:31:29: error: initializer-string for array of chars is too long [-Werror] 31 | [ERRCODE_OFFSET(VERIFY)] = "Kernel verifier blocks program loading", | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ libbpf_errno.c:31:29: note: (near initialization for ‘libbpf_strerror_table[7]’) libbpf_errno.c:34:31: error: initializer-string for array of chars is too long [-Werror] 34 | [ERRCODE_OFFSET(PROGTYPE)] = "Kernel doesn't support this program type", | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ libbpf_errno.c:34:31: note: (near initialization for ‘libbpf_strerror_table[10]’) libbpf_errno.c:37:30: error: initializer-string for array of chars is too long [-Werror] 37 | [ERRCODE_OFFSET(NLPARSE)] = "Incorrect netlink message parsing", | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ libbpf_errno.c:37:30: note: (near initialization for ‘libbpf_strerror_table[13]’) cc1: all warnings being treated as errors make: *** [<builtin>: libbpf_errno.o] Error 1
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index 2a82f49ce16f..2e5df1624f79 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -265,8 +265,6 @@ static void pr_perm_msg(int err) buf); }
-#define STRERR_BUFSIZE 128 - /* Copied from tools/perf/util/util.h */ #ifndef zfree # define zfree(ptr) ({ free(*ptr); *ptr = NULL; }) diff --git a/tools/lib/bpf/libbpf_errno.c b/tools/lib/bpf/libbpf_errno.c index 96f67a772a1b..2f03f861b8b6 100644 --- a/tools/lib/bpf/libbpf_errno.c +++ b/tools/lib/bpf/libbpf_errno.c @@ -21,7 +21,7 @@ #define ERRCODE_OFFSET(c) ERRNO_OFFSET(LIBBPF_ERRNO__##c) #define NR_ERRNO (__LIBBPF_ERRNO__END - __LIBBPF_ERRNO__START)
-static const char *libbpf_strerror_table[NR_ERRNO] = { +static const char libbpf_strerror_table[NR_ERRNO][STRERR_BUFSIZE] = { [ERRCODE_OFFSET(LIBELF)] = "Something wrong in libelf", [ERRCODE_OFFSET(FORMAT)] = "BPF object format invalid", [ERRCODE_OFFSET(KVERSION)] = "'version' section incorrect or lost", diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h index 377642ff51fc..d4dc4fe945a6 100644 --- a/tools/lib/bpf/libbpf_internal.h +++ b/tools/lib/bpf/libbpf_internal.h @@ -57,6 +57,8 @@ #define ELF64_ST_VISIBILITY(o) ((o) & 0x03) #endif
+#define STRERR_BUFSIZE 128 + #define BTF_INFO_ENC(kind, kind_flag, vlen) \ ((!!(kind_flag) << 31) | ((kind) << 24) | ((vlen) & BTF_MAX_VLEN)) #define BTF_TYPE_ENC(name, info, size_or_type) (name), (info), (size_or_type)
| |