| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <02b9227e-81a2-54fe-9832-dadb30520004@fb.com>
Date: Wed, 17 Oct 2018 17:28:30 +0000
From: Yonghong Song <yhs@...com>
To: Edward Cree <ecree@...arflare.com>,
Alexei Starovoitov <ast@...com>, Martin Lau <kafai@...com>,
"daniel@...earbox.net" <daniel@...earbox.net>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC: Kernel Team <Kernel-team@...com>
Subject: Re: [PATCH bpf-next v2 13/13] tools/bpf: bpftool: add support for
jited func types
On 10/17/18 4:11 AM, Edward Cree wrote:
> On 17/10/18 08:24, Yonghong Song wrote:
>> This patch added support to print function signature
>> if btf func_info is available. Note that ksym
>> now uses function name instead of prog_name as
>> prog_name has a limit of 16 bytes including
>> ending '\0'.
>>
>> The following is a sample output for selftests
>> test_btf with file test_btf_haskv.o:
>>
>> $ bpftool prog dump jited id 1
>> int _dummy_tracepoint(struct dummy_tracepoint_args * ):
>> bpf_prog_b07ccb89267cf242__dummy_tracepoint:
>> 0: push %rbp
>> 1: mov %rsp,%rbp
>> ......
>> 3c: add $0x28,%rbp
>> 40: leaveq
>> 41: retq
>>
>> int test_long_fname_1(struct dummy_tracepoint_args * ):
>> bpf_prog_2dcecc18072623fc_test_long_fname_1:
>> 0: push %rbp
>> 1: mov %rsp,%rbp
>> ......
>> 3a: add $0x28,%rbp
>> 3e: leaveq
>> 3f: retq
>>
>> int test_long_fname_2(struct dummy_tracepoint_args * ):
>> bpf_prog_89d64e4abf0f0126_test_long_fname_2:
>> 0: push %rbp
>> 1: mov %rsp,%rbp
>> ......
>> 80: add $0x28,%rbp
>> 84: leaveq
>> 85: retq
>>
>> Signed-off-by: Yonghong Song <yhs@...com>
>> ---
>> tools/bpf/bpftool/btf_dumper.c | 96 ++++++++++++++++++++++++++++++++++
>> tools/bpf/bpftool/main.h | 2 +
>> tools/bpf/bpftool/prog.c | 54 +++++++++++++++++++
>> 3 files changed, 152 insertions(+)
>>
>> diff --git a/tools/bpf/bpftool/btf_dumper.c b/tools/bpf/bpftool/btf_dumper.c
>> index 55bc512a1831..a31df4202335 100644
>> --- a/tools/bpf/bpftool/btf_dumper.c
>> +++ b/tools/bpf/bpftool/btf_dumper.c
>> @@ -249,3 +249,99 @@ int btf_dumper_type(const struct btf_dumper *d, __u32 type_id,
>> {
>> return btf_dumper_do_type(d, type_id, 0, data);
>> }
>> +
>> +#define BTF_PRINT_STRING(str) \
>> + { \
>> + pos += snprintf(func_sig + pos, size - pos, str); \
>> + if (pos >= size) \
>> + return -1; \
>> + }
> Usual kernel practice for this sort of macro is to use
> do { \
> } while(0)
> to ensure correct behaviour if the macro is used within another control
> flow statement, e.g.
> if (x)
> BTF_PRINT_STRING(x);
> else
> do_something_else();
> will not compile with the bare braces as the else will be detached.
Thanks for the review! Will change to use the "do while" format
as you suggested.
>> +#define BTF_PRINT_ONE_ARG(fmt, arg) \
>> + { \
>> + pos += snprintf(func_sig + pos, size - pos, fmt, arg); \
>> + if (pos >= size) \
>> + return -1; \
>> + }
> Any reason for not just using a variadic macro?
No particular reason. I will try to use it in the next revision.
>> +#define BTF_PRINT_TYPE_ONLY(type) \
>> + { \
>> + pos = __btf_dumper_type_only(btf, type, func_sig, \
>> + pos, size); \
>> + if (pos == -1) \
>> + return -1; \
>> + }
>> +
>> +static int __btf_dumper_type_only(struct btf *btf, __u32 type_id,
>> + char *func_sig, int pos, int size)
>> +{
>> + const struct btf_type *t = btf__type_by_id(btf, type_id);
>> + const struct btf_array *array;
>> + int i, vlen;
>> +
>> + switch (BTF_INFO_KIND(t->info)) {
>> + case BTF_KIND_INT:
>> + BTF_PRINT_ONE_ARG("%s ",
>> + btf__name_by_offset(btf, t->name_off));
>> + break;
>> + case BTF_KIND_STRUCT:
>> + BTF_PRINT_ONE_ARG("struct %s ",
>> + btf__name_by_offset(btf, t->name_off));
>> + break;
>> + case BTF_KIND_UNION:
>> + BTF_PRINT_ONE_ARG("union %s ",
>> + btf__name_by_offset(btf, t->name_off));
>> + break;
>> + case BTF_KIND_ENUM:
>> + BTF_PRINT_ONE_ARG("enum %s ",
>> + btf__name_by_offset(btf, t->name_off));
>> + break;
>> + case BTF_KIND_ARRAY:
>> + array = (struct btf_array *)(t + 1);
>> + BTF_PRINT_TYPE_ONLY(array->type);
>> + BTF_PRINT_ONE_ARG("[%d]", array->nelems);
>> + break;
>> + case BTF_KIND_PTR:
>> + BTF_PRINT_TYPE_ONLY(t->type);
>> + BTF_PRINT_STRING("* ");
>> + break;
>> + case BTF_KIND_UNKN:
>> + case BTF_KIND_FWD:
>> + case BTF_KIND_TYPEDEF:
>> + return -1;
>> + case BTF_KIND_VOLATILE:
>> + BTF_PRINT_STRING("volatile ");
>> + BTF_PRINT_TYPE_ONLY(t->type);
>> + break;
>> + case BTF_KIND_CONST:
>> + BTF_PRINT_STRING("const ");
>> + BTF_PRINT_TYPE_ONLY(t->type);
>> + break;
>> + case BTF_KIND_RESTRICT:
>> + BTF_PRINT_STRING("restrict ");
>> + BTF_PRINT_TYPE_ONLY(t->type);
>> + break;
>> + case BTF_KIND_FUNC:
>> + case BTF_KIND_FUNC_PROTO:
>> + BTF_PRINT_TYPE_ONLY(t->type);
>> + BTF_PRINT_ONE_ARG("%s(", btf__name_by_offset(btf, t->name_off));
>> + vlen = BTF_INFO_VLEN(t->info);
>> + for (i = 0; i < vlen; i++) {
>> + __u32 arg_type = ((__u32 *)(t + 1))[i];
>> +
>> + BTF_PRINT_TYPE_ONLY(arg_type);
>> + if (i != (vlen - 1))
>> + BTF_PRINT_STRING(", ");
>> + }
> In this kind of loop I find it cleaner to print the comma before the item;
> that way the test becomes i != 0. Thus:
> for (i = 0; i < vlen; i++) {
> __u32 arg_type = ((__u32 *)(t + 1))[i];
>
> if (i)
> BTF_PRINT_STRING(", ");
> BTF_PRINT_TYPE_ONLY(arg_type);
> }
Good suggestion. Will make change in the next revision.
>
> -Ed
>
Powered by blists - more mailing lists